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

Issue 272 implement #357

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

milssky
Copy link

@milssky milssky commented Jul 24, 2024

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

There are couple of main things we need to address before I can even start reviewing it:

  • This should be off by default, this can break users' stuff. Since behaviour change like this can be very hard to debug
  • We need a CLI flag to enable this feature
  • We need to find some other way to evaluate variables, re is not good enough for that

Copy link

codecov bot commented Jul 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (6bd75f4) to head (0ed121c).
Report is 29 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #357   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           83        96   +13     
  Branches        18        21    +3     
=========================================
+ Hits            83        96   +13     

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

@milssky
Copy link
Author

milssky commented Sep 15, 2024

There are couple of main things we need to address before I can even start reviewing it:

  • This should be off by default, this can break users' stuff. Since behaviour change like this can be very hard to debug
  • We need a CLI flag to enable this feature
  • We need to find some other way to evaluate variables, re is not good enough for that

Done.
The code without regex some ugly =D

@milssky milssky requested a review from sobolevn September 15, 2024 19:24
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.

2 participants