-
Notifications
You must be signed in to change notification settings - Fork 0
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
Library guidelines #36
Labels
Comments
@SimonLab excellent doc reading, thanks! 👌🏻 |
nelsonic
added a commit
that referenced
this issue
May 13, 2022
nelsonic
added a commit
that referenced
this issue
May 13, 2022
Initial PR with the “easy” part: name spacing. 🤞 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Review guidelines for building library: https://hexdocs.pm/elixir/main/library-guidelines.html
Check for possible error inside the library:
For example:
gogs/lib/gogs.ex
Lines 162 to 165 in 2c55a5b
Here we call the
File.read
function without checking for errors. We can add a case statement here to make sure we read the file without any errors or report the error from the library itself to make is easier to debugsee: https://hexdocs.pm/elixir/main/library-guidelines.html#avoid-working-with-invalid-data
Application configuration
At the moment we are using module constant to defined from environment variable. We had a few issues linked to this and it might be better to let the application using the library define these values in the config file.
see https://hexdocs.pm/elixir/main/library-guidelines.html#avoid-application-configuration
Rename modules
Create the
Gogs
namespace and use it for other modules:gogs/lib/helpers.ex
Line 1 in 2c55a5b
become
Gogs.Helpers
see: https://hexdocs.pm/elixir/main/library-guidelines.html#avoid-defining-modules-that-are-not-in-your-namespace
These are the main points that we can update for now.
The text was updated successfully, but these errors were encountered: