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

infra(unicorn): escape-case #2469

Merged
merged 7 commits into from
Oct 17, 2023
Merged

infra(unicorn): escape-case #2469

merged 7 commits into from
Oct 17, 2023

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Oct 10, 2023

@ST-DDT ST-DDT added p: 1-normal Nothing urgent c: infra Changes to our infrastructure or project setup labels Oct 10, 2023
@ST-DDT ST-DDT added this to the vAnytime milestone Oct 10, 2023
@ST-DDT ST-DDT requested review from a team October 10, 2023 22:07
@ST-DDT ST-DDT self-assigned this Oct 10, 2023
@ST-DDT ST-DDT added the s: needs decision Needs team/maintainer decision label Oct 10, 2023
@ST-DDT
Copy link
Member Author

ST-DDT commented Oct 10, 2023

I'm not sure about this rule, because our numbers use lowercase hex characters.

@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

Merging #2469 (c6ff3c9) into next (b8f31f6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #2469   +/-   ##
=======================================
  Coverage   99.58%   99.58%           
=======================================
  Files        2823     2823           
  Lines      255517   255517           
  Branches     1101     1103    +2     
=======================================
+ Hits       254463   254466    +3     
+ Misses       1026     1023    -3     
  Partials       28       28           
Files Coverage Δ
src/modules/git/index.ts 100.00% <100.00%> (ø)
src/modules/helpers/index.ts 98.98% <100.00%> (ø)
src/modules/internet/index.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@xDivisionByZerox
Copy link
Member

[...] our numbers use lowercase hex characters.

Maybe we should discuss if that is the expected behavior. There is propably a ISO or RFC that standardizes representation of those values.

Copy link
Member

@xDivisionByZerox xDivisionByZerox left a comment

Choose a reason for hiding this comment

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

I don't see a problem with the rule as it only verifies static escaped characters. I see that there would be a mismatch between the values we use in our code base and the values we provide for users, but I would not consider this a different topic of discussion.

@ST-DDT ST-DDT removed the s: needs decision Needs team/maintainer decision label Oct 10, 2023
@ST-DDT ST-DDT enabled auto-merge (squash) October 17, 2023 22:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
c: infra Changes to our infrastructure or project setup p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants