Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define status as struct #3

Merged
merged 2 commits into from
Jul 9, 2022
Merged

Define status as struct #3

merged 2 commits into from
Jul 9, 2022

Conversation

SimonLab
Copy link
Member

@SimonLab SimonLab commented Jul 8, 2022

use Struct to define the list of statuses, remove json file and File.read function
ref: #4

@nelsonic let me know your thought on this,thanks

- Define struct Status
- Return list of status directly from Statuses module
instead of reading a json file
- add a function to return statuses as json
@SimonLab SimonLab added the in-progress An issue or pull request that is being worked on by the assigned person label Jul 8, 2022
@SimonLab SimonLab self-assigned this Jul 8, 2022
Update Readme and mix.exs config file for package
@codecov
Copy link

codecov bot commented Jul 8, 2022

Codecov Report

Merging #3 (2b9aa34) into main (f6783e0) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main        #3   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            4         2    -2     
=========================================
- Hits             4         2    -2     
Impacted Files Coverage Δ
lib/statuses.ex 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f6783e0...2b9aa34. Read the comment docs.

@SimonLab SimonLab mentioned this pull request Jul 8, 2022
3 tasks
@SimonLab SimonLab requested a review from nelsonic July 8, 2022 13:52
@SimonLab SimonLab added awaiting-review An issue or pull request that needs to be reviewed and removed in-progress An issue or pull request that is being worked on by the assigned person labels Jul 8, 2022
@@ -23,10 +23,6 @@ jobs:
path: deps
key: ${{ runner.os }}-mix-${{ hashFiles('**/mix.lock') }}
restore-keys: ${{ runner.os }}-mix-
- name: Install Node.js Deps (JSON Lint)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the PR doesn't have the json file, we don't need to install npm pacakage

"""
@type t :: %Status{code: integer(), desc: String.t(), text: atom()}
@enforce_keys [:code, :desc, :text]
@derive Jason.Encoder
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need to encode some struct that does not implement the protocol, if you own the struct, you can derive the implementation specifying which fields should be encoded to JSON

https://github.com/michalmuskala/jason#differences-to-poison

Define the Status struct with desc, id and text key required
"""
@type t :: %Status{code: integer(), desc: String.t(), text: atom()}
@enforce_keys [:code, :desc, :text]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all the keys are required

@doc """
Returns the list of statuses as json
"""
def statuses_to_json(), do: Jason.encode!(get_statuses())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create a function to return json, I don't think we'll use it but I can see how it can be useful

@SimonLab SimonLab marked this pull request as ready for review July 8, 2022 13:57
Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonLab looks good. 👍

Detailed response in the issue: #4 (comment)

@nelsonic nelsonic merged commit b3c5950 into main Jul 9, 2022
@nelsonic nelsonic deleted the cleanup branch July 9, 2022 21:27
@nelsonic
Copy link
Member

nelsonic commented Jul 9, 2022

Package published to https://hex.pm/packages/statuses/1.1.0 📦 🚀

@SimonLab SimonLab removed their assignment Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review An issue or pull request that needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants