Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add @file directive #1048

Closed
wants to merge 14 commits into from
Closed

Conversation

mogery
Copy link
Contributor

@mogery mogery commented Jan 27, 2024

Summary:
Adds the @file directive, like described in #932. Uses compile_const under the hood for max code reuse and to make sure we only read a file once.

Issue Reference(s):
Fixes #932
/claim #932

Also partly addresses #764

Build & Testing:

  • I ran cargo test successfully.
  • I have run ./lint.sh --mode=fix to fix all linting issues raised by ./lint.sh --mode=check.

Checklist:

@github-actions github-actions bot added the type: feature Brand new functionality, features, pages, workflows, endpoints, etc. label Jan 27, 2024
@mogery
Copy link
Contributor Author

mogery commented Jan 29, 2024

This is ready now as well.

Copy link

codecov bot commented Jan 29, 2024

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (0d300f3) 88.45% compared to head (939fc4b) 88.50%.
Report is 25 commits behind head on main.

Files Patch % Lines
src/blueprint/operators/file.rs 89.13% 5 Missing ⚠️
src/config/expr.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1048      +/-   ##
==========================================
+ Coverage   88.45%   88.50%   +0.04%     
==========================================
  Files         104      105       +1     
  Lines       11161    11226      +65     
==========================================
+ Hits         9873     9936      +63     
- Misses       1288     1290       +2     

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

@mogery
Copy link
Contributor Author

mogery commented Jan 30, 2024

@meskill Do the codecov issues on this make sense? I think the testing is good enough. Not sure why the missed lines are missed.

@meskill
Copy link
Contributor

meskill commented Jan 30, 2024

@meskill Do the codecov issues on this make sense? I think the testing is good enough. Not sure why the missed lines are missed.

They do. You can check the report or annotations in the pr. It mostly about some invalid cases check and usage of file inside @expr

@mogery
Copy link
Contributor Author

mogery commented Jan 30, 2024

@meskill Do the codecov issues on this make sense? I think the testing is good enough. Not sure why the missed lines are missed.

They do. You can check the report or annotations in the pr. It mostly about some invalid cases check and usage of file inside @expr

ohhhhh! thanks. i didn't even think about @expr

@mogery
Copy link
Contributor Author

mogery commented Jan 30, 2024

Alright, added tests for expr as well.

@tusharmath tusharmath added the ci: lint Automatically fix the linters issues and make a commit label Feb 4, 2024
@mogery mogery force-pushed the mog/file-directive branch from 1b9daa0 to 8445b88 Compare February 4, 2024 13:52
Copy link

github-actions bot commented Feb 8, 2024

Action required: PR inactive for 2 days.
Status update or closure in 5 days.

@github-actions github-actions bot added the state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. label Feb 8, 2024
@mogery
Copy link
Contributor Author

mogery commented Feb 8, 2024

Closing this PR as reworking it to use @link and @const will cause the code to be much different. @tusharmath Could you assign me to #932 and #764 as well? The new PR will solve both issues.

@mogery mogery closed this Feb 8, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🙋 Bounty claim ci: lint Automatically fix the linters issues and make a commit state: inactive No current action needed/possible; issue fixed, out of scope, or superseded. type: feature Brand new functionality, features, pages, workflows, endpoints, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature: add a @file operator
3 participants