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

Multifits #110

Closed
wants to merge 126 commits into from
Closed

Multifits #110

wants to merge 126 commits into from

Conversation

mpound
Copy link
Collaborator

@mpound mpound commented Nov 16, 2023

Clean merge of multifits into release 0.2.0

mpound and others added 30 commits June 19, 2023 11:48
Fix syntax error in release.yaml

Can't say for sure that the release action works as intended, but it at least appears to have stopped causing syntax errors
Hatch:
- Add test envs
- Add [dev] and [docs] optional-dependencies "features"
- Add new scripts for easy linting/testing/doc-building

Linting/tooling:
- Expand ruff linting rule selections
- Replace isort config; replace with ruff
- Remove pyright config

pre-commit:
- No longer using isort; replaced with ruff
- Tweaks to black args
- Update pip-compile step so it works with pyproject.toml (installs all
optional dependencies and dumps them to requirements.txt)
- Run $ pre-commit auto-update
Note: there are currently 27 linting errors from ruff. I am leaving
these in here; they will need to be either fixed, or skipped using '$ SKIP=ruff git commit
...' before further commits can be made
Tweak a few linting settings
Disable ruff pre-commit hook in github action, since it is currently
failing
It's currently failing due to issues that I'm not qualified to fix

Also, merged docs and dev in optional-dependencies

Renamed matrix test runner
These changes look great, thanks for all the work!
Adds Evan's development branch (evan-devel) to the list of triggering branches.
astrofle and others added 26 commits November 13, 2023 03:19
@mpound mpound requested a review from vcatlett November 16, 2023 18:29
@tchamberlin
Copy link
Member

There are two issues with pre-commit here:

  1. Neither multifits nor release-0.2.0 are compliant with their own pre-commit hooks. pre-commit run --all-files should be a no-op, but it results in changes on both of these branches
  2. .pre-commit-config.yaml is not identical between these two branches

A potential path forward:

  1. Create release-0.2.0-tmp, off of current release-0.2.0
  2. On release-0.2.0-tmp, check out multifits version of .pre-commit-config.yaml
  3. On release-0.2.0-tmp, do pre-commit run --all-files, then commit something like "Run pre-commit on all files"
  4. On multifits, do pre-commit run --all-files, then commit something like "Run pre-commit on all files"
  5. Then start the merge

That way you have the same formatting applied on both branches, which should remove any formatting-specific diffs/conflicts

Note that this will only be an issue short term, because we still don't have the pre-commit hooks run/enforced on all branches

@mpound
Copy link
Collaborator Author

mpound commented Nov 16, 2023

Thanks, I will do this.

@mpound mpound closed this Nov 16, 2023
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 this pull request may close these issues.

4 participants