-
Notifications
You must be signed in to change notification settings - Fork 10
Modernize Makefile and CI workflow #615
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
Changes from 2 commits
a765347
00abe42
a05dc10
250622f
807e631
4ee2d90
4fec51a
0e89d88
3ffcab6
cedcc38
9bbb8b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,98 @@ | ||||||||||||||||||||||||||||||||||||||
| # Contributing to Firecrown | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Thank you for your interest in contributing to Firecrown! This document provides guidelines and workflows to help you get started. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| ## Development Environment Setup | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| We recommend using `conda` (or `mamba`/`miniforge`) to manage your development environment. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| 1. **Clone the repository:** | ||||||||||||||||||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||||||||||||||||||
| git clone https://github.com/LSSTDESC/firecrown.git | ||||||||||||||||||||||||||||||||||||||
| cd firecrown | ||||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| 2. **Create and activate the environment:** | ||||||||||||||||||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||||||||||||||||||
| conda env create -f environment.yml | ||||||||||||||||||||||||||||||||||||||
| conda activate firecrown_developer | ||||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| 3. **Install Firecrown in development mode:** | ||||||||||||||||||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||||||||||||||||||
| make install | ||||||||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| ## Recommended Developer Workflow | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| To maintain high code quality and consistency, we use several automated tools. We recommend following this workflow during development: | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| | Target | Description | When to run | | ||||||||||||||||||||||||||||||||||||||
| | :--- | :--- | :--- | | ||||||||||||||||||||||||||||||||||||||
| | `make format` | Automatically format all code using `black` | Frequently during development | | ||||||||||||||||||||||||||||||||||||||
| | `make lint -j` | Run all linters (`black`, `flake8`, `mypy`, `pylint`) in parallel | Before every commit | | ||||||||||||||||||||||||||||||||||||||
| | `make test -j` | Run fast unit tests in parallel | Regularly during development | | ||||||||||||||||||||||||||||||||||||||
| | `make unit-tests -j` | Run all unit tests with 100% per-component coverage check | Before pushing | | ||||||||||||||||||||||||||||||||||||||
| | `make test-ci` | Run the full test suite exactly as the CI system does | Final check before pushing | | ||||||||||||||||||||||||||||||||||||||
| | `make docs -j` | Build and verify all documentation (tutorials + API) | When changing tutorials or docstrings | | ||||||||||||||||||||||||||||||||||||||
| | `make pre-commit` | A comprehensive check: format, lint, docs-verify, and full tests | Recommended pre-push check | | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| > [!TIP] | ||||||||||||||||||||||||||||||||||||||
| > Always use the `-j` flag (e.g., `make -j lint`) to take full advantage of parallel execution. The `Makefile` automatically detects the number of available CPUs. | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
| | `make lint -j` | Run all linters (`black`, `flake8`, `mypy`, `pylint`) in parallel | Before every commit | | |
| | `make test -j` | Run fast unit tests in parallel | Regularly during development | | |
| | `make unit-tests -j` | Run all unit tests with 100% per-component coverage check | Before pushing | | |
| | `make test-ci` | Run the full test suite exactly as the CI system does | Final check before pushing | | |
| | `make docs -j` | Build and verify all documentation (tutorials + API) | When changing tutorials or docstrings | | |
| | `make pre-commit` | A comprehensive check: format, lint, docs-verify, and full tests | Recommended pre-push check | | |
| > [!TIP] | |
| > Always use the `-j` flag (e.g., `make -j lint`) to take full advantage of parallel execution. The `Makefile` automatically detects the number of available CPUs. | |
| | `JOBS=N make lint` | Run all linters (`black`, `flake8`, `mypy`, `pylint`) in parallel | Before every commit | | |
| | `JOBS=N make test` | Run fast unit tests in parallel | Regularly during development | | |
| | `JOBS=N make unit-tests` | Run all unit tests with 100% per-component coverage check | Before pushing | | |
| | `make test-ci` | Run the full test suite exactly as the CI system does | Final check before pushing | | |
| | `JOBS=N make docs` | Build and verify all documentation (tutorials + API) | When changing tutorials or docstrings | | |
| | `make pre-commit` | A comprehensive check: format, lint, docs-verify, and full tests | Recommended pre-push check | | |
| > [!TIP] | |
| > The `Makefile` enables parallel execution by default, so `make <target>` will already use multiple CPUs where appropriate. If you want to control the level of parallelism, set an explicit job count, for example `JOBS=4 make lint` or `make -j4 lint`, rather than using bare `-j`. |
Copilot
AI
Feb 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This refers to submitting PRs to the master branch, but the repository appears to use main as the default branch (PR diffs are against main). Update this to the correct default branch name to avoid confusing contributors.
| 4. **Submit PR**: Once your tests pass locally, submit a Pull Request to the `master` branch. | |
| 4. **Submit PR**: Once your tests pass locally, submit a Pull Request to the `main` branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In non-coverage matrix runs, this step executes
make test-all, andtest-allalready includestest-examplein the Makefile. The subsequent “Running example tests” step runsmake test-exampleagain, duplicating work/time. Either remove the separate example step or change the non-coverage branch to run a target that excludes examples.