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

Add CONTRIBUTING.md #631

Closed
valerauko opened this issue Oct 6, 2022 · 2 comments · Fixed by #633
Closed

Add CONTRIBUTING.md #631

valerauko opened this issue Oct 6, 2022 · 2 comments · Fixed by #633

Comments

@valerauko
Copy link
Contributor

Having a CONTRIBUTING.md would help a lot IMO especially with Hacktoberfest.

Aleph is a pretty big and complicated codebase (especially if you try to understand the execution flow) so having a rough guide on

  • how to set up a local dev environment
  • what tests/lints to run before commit/push
  • what's the general workflow once PR is made
  • is it okay to touch the ci pipeline (improve/add/tweak jobs etc)
  • how to handle non-app-code cases like dependency updates
  • how to handle non-code cases like documentation improvements
@KingMob
Copy link
Collaborator

KingMob commented Oct 8, 2022

Thanks to @valerauko for kicking this off and providing a first pass.

Before we close the issue, maybe we can hash out the rest of their list, and add anything else we think is important. I'll jot down what we have so far, please correct me if I'm wrong.

  • Local dev environment
    • Other than Leiningen, is there anything we require?
  • Tests/lints before commit/push
    • To save everyone time, lein test should be run, and pass, before pushing up to GH
    • clj-kondo is recommended for linting. Between potemkin and various proto-potemkin macros, Aleph will trigger a lot of false positives, and kondo is the least bad at understanding them. Fixing this is ongoing. (I recently releaesd an update to Potemkin that helps a bit, but there's still plenty of work to be done here)
    • If you are fixing a bug, create a test that demonstrates it first, then make the fix. This ensures our understanding of the problem is correct.
  • PR workflow
    • When ready, make a PR. @arnaudgeiser, @DerGuteMoritz and I will get automatically added as reviewers
    • Typically, we will discuss it a bit. Currently, a single approval is sufficient, if nobody objects or requests changes.
    • (Side note: it would be nice to suggest/mandate access to other people's branches, so reviewers can more easily change PR code. Is there a way to do this?)
    • After approval, I may ask you to clean up history and force-push, if it's messy
    • One of the reviewers will then merge
    • Releases should be independent of PRs (i.e., a PR should not also create a new version)
  • CI changes
    • Updating CI is fine, but coordinate with Erik Assum (slipset), who built a consistent set of CircleCI scripts/configs for all the clj-commons repos.
  • Dependency updates
    • Captured in current CONTRIBUTING.md doc. We have a script to run to support deps.edn users.
    • (A way to automate that in a non-onerous manner would be nice.)
  • Documentation
    • I would like to transition away from the literate doc aleph.io site (since we don't have access to it), and towards cljdoc.org. To that end, new docs should be cljdoc.org compatible (metadata, article formats, etc)

Thoughts?

@arnaudgeiser
Copy link
Collaborator

arnaudgeiser commented Oct 8, 2022

Local dev environment
Other than Leiningen, is there anything we require?

As far as I know, that's the only "tooling" requirement.
On my side, I also require the JDK to be Java 8 otherwise I'm not able to
compile some parts of the code.
But some efforts can be done on that front to make aleph compile whatever
the JDK is.

If you are fixing a bug, create a test that demonstrates it first, then make the fix. This ensures our understanding of the problem is correct.

Yes, and I think we have to be very strict about it.
I would even say for the new features where it can be tested easily.

I would like to transition away from the literate doc aleph.io site (since we don't have access to it), and towards cljdoc.org. To that end, new docs should be cljdoc.org compatible (metadata, article formats, etc)

While I really like the current literate doc we expose (it forces us to have docstrings that make sense), I concur.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants