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

ENH: Add CLI tool for filename validation for use in pre-receive hooks #1700

Closed
wants to merge 8 commits into from

Conversation

effigies
Copy link
Collaborator

@effigies effigies commented Feb 9, 2024

This is a basic validator that accepts a .bidsignore list and a list of files. The use case is to quickly determine if a git push should be accepted by a server.

Todo:

  • Accept dataset_description.json from stdin, and enable derivatives
  • More efficient fnmatch filtering
  • Consider streaming stdin instead of slurping the entire input

@effigies effigies added schema-code Updates or changes to the code used to parse, filter, and render the schema. exclude-from-changelog This item will not feature in the automatically generated changelog labels Feb 9, 2024
@effigies
Copy link
Collaborator Author

@nellh What would be a good format for dataset_description? Right now it's:

ignore-entry1
ignore-entry2
...
ignore-entryN
0001
file1
file2
...
fileN

We could do:

{
  ...
}
0000
ignore-entry1
ignore-entry2
...
ignore-entryN
0001
file1
file2
...
fileN

This would be kinda backwards compatible, as the 0001 will continue to separate ignore and file listing, and then we could look for 0000\n in the ignore list. But I'm not sure if we care about backwards compatibility, and if there might be something better.

Copy link

codecov bot commented Feb 29, 2024

Codecov Report

Attention: Patch coverage is 1.47059% with 67 lines in your changes are missing coverage. Please review.

Project coverage is 83.89%. Comparing base (bd08602) to head (087ccb4).

❗ Current head 087ccb4 differs from pull request most recent head ec42201. Consider uploading reports for the commit ec42201 to get more accurate results

Files Patch % Lines
tools/schemacode/bidsschematools/__main__.py 0.00% 67 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1700      +/-   ##
==========================================
- Coverage   87.93%   83.89%   -4.04%     
==========================================
  Files          16       16              
  Lines        1351     1416      +65     
==========================================
  Hits         1188     1188              
- Misses        163      228      +65     

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

The 0001 is a particular git-protocol-ism, and 0000 does not behave as I
expected. Instead of adding multiple 0001s and attempting to identify
the meaning, the new protocol preceeds the old protocol with a header
line and a single JSON line.
Copy link
Member

@nellh nellh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@effigies I merged this too soon on the OpenNeuro side but looks fine with a couple fixes (see suggested changes). Anything else we should address to merge this?

tools/schemacode/bidsschematools/__main__.py Outdated Show resolved Hide resolved
tools/schemacode/bidsschematools/__main__.py Outdated Show resolved Hide resolved
@effigies effigies closed this Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude-from-changelog This item will not feature in the automatically generated changelog schema-code Updates or changes to the code used to parse, filter, and render the schema.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants