|
| 1 | +# Contributing to Permit.Ecto |
| 2 | + |
| 3 | +Thank you for your interest in contributing to Permit.Ecto! This document provides guidelines and information for contributors. |
| 4 | + |
| 5 | +## Code of Conduct |
| 6 | + |
| 7 | +This project adheres to a [Code of Conduct](CODE_OF_CONDUCT.md). By participating, you are expected to uphold this code. |
| 8 | + |
| 9 | +## Getting started |
| 10 | + |
| 11 | +### Development environment |
| 12 | + |
| 13 | +#### Prerequisites |
| 14 | + |
| 15 | +* Elixir 1.12+ and OTP 24+ |
| 16 | + - _If implementing a proposed feature requires bumping Elixir or OTP, we're open to considering that._ |
| 17 | +* Git |
| 18 | +* **PostgreSQL** - Required for running tests and development of Ecto-dependent features |
| 19 | + |
| 20 | +Developing and testing the `permit_ecto` package requires a running Postgres instance as this package provides database integration for Permit's authorization system. |
| 21 | + |
| 22 | +#### Setup |
| 23 | + |
| 24 | +1. Clone the repository and install dependencies: |
| 25 | + ```bash |
| 26 | + git clone https://github.com/curiosum-dev/permit_ecto.git |
| 27 | + cd permit_ecto |
| 28 | + mix deps.get |
| 29 | + ``` |
| 30 | + |
| 31 | +2. Set up your test database (configure in `config/test.exs`): |
| 32 | + ```bash |
| 33 | + MIX_ENV=test mix do ecto.create, ecto.migrate |
| 34 | + ``` |
| 35 | + |
| 36 | +3. Run tests to ensure everything is working: |
| 37 | + ```bash |
| 38 | + mix test |
| 39 | + ``` |
| 40 | + |
| 41 | +When running Credo and Dialyzer, please use `MIX_ENV=test` to ensure tests and support files are validated, too. |
| 42 | + |
| 43 | +## How to contribute |
| 44 | + |
| 45 | +### Before proceeding: where to discuss things? |
| 46 | + |
| 47 | +The best place to discuss fresh ideas for the library's future and ask any sorts of questions (not strictly constituting an issue as defined below) is the [Permit channel in Elixir's Slack workspace][0]. You can also use the appropriate [GitHub Discussions][1] channel. We're usually easiest approachable via Slack, though 😉 |
| 48 | + |
| 49 | +For example, if you've got a general question about Permit.Ecto's development direction, or about its intended behaviour in a specific scenario, it's more convenient to open up a discussion on Slack (or use GitHub discussions) than to file an issue right away. |
| 50 | + |
| 51 | +However, if there's clearly a bug you'd like to report, or you have a specific feature idea that you'd like to explain, it's perfect material for a GitHub issue inside the project! |
| 52 | + |
| 53 | +### Reporting issues |
| 54 | + |
| 55 | +Before creating an issue, please: |
| 56 | + |
| 57 | +1. **Search existing issues** to avoid duplicates |
| 58 | +2. **Use the issue templates** when available |
| 59 | +3. **For bug reports, provide detailed information**: |
| 60 | + - Elixir/OTP versions |
| 61 | + - Permit.Ecto version |
| 62 | + - Ecto and Postgres versions |
| 63 | + - Minimal reproduction case |
| 64 | + - Expected vs actual behavior |
| 65 | +4. **For feature requests, check against roadmap outlined in [main Permit README](https://github.com/curiosum-dev/permit/blob/master/README.md) and provide the following**: |
| 66 | + - Purpose of the new feature |
| 67 | + - Intended usage syntax (or pseudocode) |
| 68 | + - Expected implementation complexity (if possible to gauge) |
| 69 | + - Expected impact on other existing features and backward compatibility |
| 70 | + - How it relates to Ecto query generation and authorization scoping |
| 71 | + |
| 72 | +### Pull requests |
| 73 | + |
| 74 | +#### Before you start |
| 75 | + |
| 76 | +- **Check existing PRs** to avoid duplicate work |
| 77 | +- **Open an issue** for discussion on significant changes (we follow the 1 issue <-> 1 PR principle) |
| 78 | +- **Follow our coding standards** (see below) |
| 79 | + |
| 80 | +#### PR process |
| 81 | + |
| 82 | +1. **Fork the repository** and create a feature branch |
| 83 | +2. **Make your changes** with clear, focused commits |
| 84 | +3. **Add tests** for new functionality |
| 85 | +4. **Update documentation** as needed, including README |
| 86 | +5. **Run the full test suite** as well as all static checks |
| 87 | +6. **Submit your PR** with a clear description |
| 88 | + |
| 89 | +#### PR Requirements |
| 90 | + |
| 91 | +- [ ] Code compiles without warnings (`mix compile --warnings-as-errors`) |
| 92 | +- [ ] Tests pass (`mix test`) |
| 93 | + - _Includes added tests to new features and fixed bugs._ |
| 94 | + - _Tests pass for all combinations of tool versions covered by the CI matrix (see `.github/workflows/elixir.yml`)._ |
| 95 | +- [ ] Code follows style guidelines (`MIX_ENV=test mix credo`) |
| 96 | + - _Run Credo in test environment to ensure tests and support code adheres to style rules as well._ |
| 97 | +- [ ] Types are correct (`MIX_ENV=test mix dialyzer`) |
| 98 | + - _Run Credo in test environment to ensure tests and support code has correct typing as well._ |
| 99 | + - _Precise typing is encouraged for all newly added modules and functions._ |
| 100 | + - _Ignores are permitted with an inline comment with proper justification._ |
| 101 | +- [ ] Documentation is updated: a bare minimum is updates to affected public API parts |
| 102 | + - _Functions intended for usage by application code must have descriptions of arguments, purpose, and usage examples._ |
| 103 | + - _For crucial additions to features, README must also be updated._ |
| 104 | +- [ ] Database tests include proper cleanup and isolation |
| 105 | +- [ ] Commit messages are clear and descriptive |
| 106 | + - If already used throughout commit history, use [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/). |
| 107 | + - Otherwise, use single-line messages formatted as: `Commit purpose in imperative mode [#issue_number]`. |
| 108 | + - Focus on what vs how, and keep it simple |
| 109 | + - Example: `Fix incorrect query generation for has_many associations [#123]` |
| 110 | + |
| 111 | +## Development guidelines |
| 112 | + |
| 113 | +### Code style |
| 114 | + |
| 115 | +We use [Credo](https://github.com/rrrene/credo) for code analysis: |
| 116 | + |
| 117 | +```bash |
| 118 | +MIX_ENV=test mix credo |
| 119 | +``` |
| 120 | + |
| 121 | +Key principles: |
| 122 | +- **Clarity over cleverness** - write readable code and avoid introducing new DSLs as much as possible. Permit.Ecto focuses on clean Ecto query generation from authorization rules. Prefer pure Elixir for easier debugging and code clarity. Use macros wisely and only when needed. |
| 123 | +* **Minimal dependencies** - Permit.Ecto should only rely on what's needed for its Ecto integration scope. Core Permit functionality belongs in the main `permit` package, while Phoenix-specific features belong in `permit_phoenix`. |
| 124 | +- **Follow Elixir conventions** - use standard patterns and don't fight the platform. Avoid [common Elixir anti-patterns](https://hexdocs.pm/elixir/what-anti-patterns.html). Write functional, idiomatic Elixir code. |
| 125 | +- **Follow Ecto conventions** - use Ecto query patterns appropriately, handle dynamic queries safely, and ensure proper query composition. |
| 126 | +- **Avoid RDBMS-specific features** - whenever possible, implement features in a way that's independent on whether Postgres, MySQL or any other DB is used. |
| 127 | +- **Document public APIs** - include `@doc` and `@moduledoc` written as if you were the one who is to read them. Avoid `@moduledoc false` unless a module is deeply private. As long as typing in Elixir relies on Dialyzer, do type with `@spec` extensively, use `Permit.Types` and `Permit.Ecto.Types` wherever applicable, and avoid typing with the likes of `map()`, `any()`, or `term()` where possible. |
| 128 | +- **Handle errors gracefully** - always consider what's the correct scheme of error propagation. There is no globally assumed convention of whether to use error tuples or exceptions, you should use whatever feels appropriate and in line with adjacent elements of the system. |
| 129 | + |
| 130 | +### Testing |
| 131 | + |
| 132 | +- **Write tests for all new functionality and bug fixes** |
| 133 | +- **Maintain high test coverage** |
| 134 | +- **Use descriptive test names** |
| 135 | +- **Test both success and failure cases** |
| 136 | +- **Test query generation correctness** - ensure generated Ecto queries match expected authorization scoping |
| 137 | +- **Test association handling** - verify proper query generation for has_one, has_many, and belongs_to relationships |
| 138 | +- **Clean up test data** - use correct setup to ensure tests don't leave side effects in the database |
| 139 | + |
| 140 | +### Commit messages |
| 141 | + |
| 142 | +Follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) format if already used in the repository. Otherwise, write single-line messages in imperative mode and mark related issue number with [#number]. |
| 143 | + |
| 144 | +### Branching strategy |
| 145 | + |
| 146 | +Follow [Conventional Branch](https://conventional-branch.github.io/) format if already used in the repository. Otherwise, use `issue_number-issue_name` as branch names. Create feature branches off `main` or `master` and avoid a nested branch tree. |
| 147 | + |
| 148 | +## Release process |
| 149 | + |
| 150 | +Releases are handled by maintainers: |
| 151 | + |
| 152 | +1. Version bump in `mix.exs` |
| 153 | +2. Update `CHANGELOG.md` |
| 154 | +3. Create GitHub release |
| 155 | +4. Publish to Hex.pm |
| 156 | + |
| 157 | +## Community |
| 158 | + |
| 159 | +### Getting help |
| 160 | + |
| 161 | +- [Elixir Slack channel][0] - for chat, questions and general discussion |
| 162 | +- [GitHub Discussions][1] - for questions and general discussion |
| 163 | +- [GitHub Issues][2] - for bug reports and feature requests |
| 164 | +- [Curiosum Blog][3] - for updates, tutorials and other content |
| 165 | + |
| 166 | +### Recognition |
| 167 | + |
| 168 | +We are eager to publicly recognize your valuable input as a Permit.Ecto contributor! Your contributions will be highlighted in subsequent release notes, as well as blog posts announcing community contributions. |
| 169 | + |
| 170 | +## Questions? |
| 171 | + |
| 172 | +Feel free to: |
| 173 | +- Ping us on [Elixir Slack][0] |
| 174 | +- Open a [GitHub Discussion](https://github.com/curiosum-dev/permit/discussions) |
| 175 | +- Contact us at [Curiosum](https://curiosum.com/contact) |
| 176 | +- Check our [blog](https://curiosum.com/blog) for updates |
| 177 | + |
| 178 | +Thank you for contributing to Permit.Ecto! 🎉 |
| 179 | + |
| 180 | +[0]: https://elixir-lang.slack.com/archives/C091Q5S0GDU |
| 181 | +[1]: https://github.com/curiosum-dev/permit/discussions |
| 182 | +[2]: https://github.com/curiosum-dev/permit_ecto/issues |
| 183 | +[3]: https://curiosum.com/blog?search=permit |
0 commit comments