Replace `File.read` used in package
- Dominant language
- Elixir
- Stars
- 5
- Forks
- 1
- PR merge metrics
- No merged PRs in 30d
Description
I had a better look at the `statuses` code and I think we can make the dependency easier to read/use:
The following `case` is bothering me:
https://github.com/dwyl/statuses/blob/f6783e0ea99cb7318e3b41d7bddfbb5dd83c0fef/lib/statuses.ex#L28-L39
- This case is here to be able to distinguish when the code is run in test and when it is used in another application as a dependency, however it took me a bit of time to understand fully this logic and add to log the `File.cwd` information to check the value on an application using the dependency
- We can have an edge case where the application using the dependency has a file path containing `/statuses`, in this case the `cwd =~ "/statuses"` will be true, however the status.json file will not be found by the application.
- We have created the `statuses.json` that we then read and convert to a list of map in Elixir. Because the dependency is only used with other Elixir application I think we can create directly the list of status map without using json. This would resolve the first point above as we won't have to use `File.read` and make sure the file path is the correct one. The main reason to have a json file is to make it easy to read and update the status, however I feel an Elixir list of map is as readable and can easily be updated, if really required we can add a small description on how to do this on the Readme.
So I proposed to refactor the package by:
- [x] Removing the json file
- [x] Creating the list of statuses directly in the `Statuses` module
- [x] Renaming the `id` to `code` to decouple the status code from the Postgres library and letting Ecto manage the link between status and the application items.
@nelsonic let me know if this makes sense, I'll add also comments on my PR #3 to try to explain a bit more my reasoning
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.