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

Run entities-service validate as a pre-commit hook #126

Closed
wants to merge 0 commits into from

Conversation

CasperWA
Copy link
Collaborator

@CasperWA CasperWA commented Apr 22, 2024

Closes #122

Add a pre-commit hook with id validate-entities.
It runs on all known file formats as well as with --verbose.

One can add extra options through the pre-commit args.
Finally, in its current version, one must add ``.[cli]'to the pre-commitadditional_dependencies`.

A separate test job has been set up to test the hook.


Important

NB: This will deprecate the --file/-f and --dir/-d options from the validate and update commands in favor of a SOURCE argument, which may be given multiple times.


Note: The commits have been squashed down to two commits, each representing the introduction of SOURCE (and deprecation of --file/-f and --dir/-d) and the new pre-commit hooks validate-entities, respectively.

Copy link

codecov bot commented Apr 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.11%. Comparing base (7b8c796) to head (49b9f0b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #126      +/-   ##
==========================================
+ Coverage   92.04%   92.11%   +0.07%     
==========================================
  Files          26       26              
  Lines        1320     1332      +12     
==========================================
+ Hits         1215     1227      +12     
  Misses        105      105              
Flag Coverage Δ
docker 80.03% <100.00%> (+0.18%) ⬆️
local 90.99% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@CasperWA CasperWA added BLOCKING Blocking issue/PR - must be fixed or merged as priority and removed BLOCKING Blocking issue/PR - must be fixed or merged as priority labels Apr 22, 2024
@CasperWA CasperWA marked this pull request as ready for review April 23, 2024 12:37
@CasperWA CasperWA requested a review from Treesarj April 23, 2024 12:37
@CasperWA CasperWA added the BLOCKING Blocking issue/PR - must be fixed or merged as priority label Apr 23, 2024
@CasperWA CasperWA force-pushed the cwa/close-122-cli-as-pre-commit-hooks branch from 4ad51ff to 268d6eb Compare April 25, 2024 07:26
@CasperWA CasperWA closed this Apr 25, 2024
@CasperWA CasperWA force-pushed the cwa/close-122-cli-as-pre-commit-hooks branch from 49b9f0b to 1ced968 Compare April 25, 2024 09:28
@CasperWA CasperWA deleted the cwa/close-122-cli-as-pre-commit-hooks branch April 25, 2024 09:28
@CasperWA
Copy link
Collaborator Author

The two commits 7bd888a and 1ced968 were what made up this PR.
Due to an extra merge commit in this PR branch' HEAD, GitHub could not recognize that this PR has been merged into main. Instead, when rebasing these commits onto main, it was found locally that this would result in a noop (indeed ...), and after pushing that, the PR has been closed.

TL;DR: This PR was successfully merged (not closed), adding the commits 7bd888a and 1ced968 to main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLOCKING Blocking issue/PR - must be fixed or merged as priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ Run some CLI commands as pre-commit hooks
2 participants