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.

Sign up for GitHub

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

Jump to bottom

Introduce %y formatter #268

Merged
merged 2 commits into from
Dec 16, 2023
Merged

Introduce %y formatter #268

merged 2 commits into from
Dec 16, 2023

Conversation

gwbres
Copy link
Collaborator

@gwbres gwbres commented Dec 16, 2023

  • Support %yy year on two digits

@gwbres
Introduced %y formatter
55242ce 
  * Support %yy year on two digits

Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
@gwbres gwbres changed the title Introduced %y formatter Introduce %y formatter Dec 16, 2023
@gwbres gwbres mentioned this pull request Dec 16, 2023
Closed
@codecov Codecov
Copy link

codecov bot commented Dec 16, 2023 •
edited
Loading

Codecov Report

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

Comparison is base (f0f7fd0) 80.74% compared to head (f4f2293) 80.73%.

Files Patch % Lines
src/efmt/formatter.rs 0.00% 2 Missing ⚠️
src/parser.rs 66.66% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #268      +/-   ##
==========================================
- Coverage   80.74%   80.73%   -0.01%     
==========================================
  Files          16       16              
  Lines        3754     3769      +15     
==========================================
+ Hits         3031     3043      +12     
- Misses        723      726       +3

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

@gwbres
add one more test
f4f2293 
Signed-off-by: Guillaume W. Bres <guillaume.bressaix@gmail.com>
@ChristopherRabotin
Copy link
Member

Looks good, thanks!

Closes #267

@ChristopherRabotin ChristopherRabotin merged commit e74c858 into nyx-space:master Dec 16, 2023
30 of 31 checks passed
gwbres
gwbres commented Dec 16, 2023
View reviewed changes
src/efmt/format.rs
decomposed[0] = sub_str
.parse::<i32>()
.map_err(|_| Errors::ParseError(ParsingErrors::ValueError))?
+ 2000;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ChristopherRabotin shouldn't sub_str be trimmed() here, so .parse() never fails ?
In other terms, may whitespaces exist prior next separator

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I didn't think of this. I forget how it works. Don't we parse token by token and therefore it isn't possible to have a space? Do the other cases have a sub str? I'm afraid I'm busy this weekend and can't look into this.

@gwbres
Copy link
Collaborator Author

gwbres commented Dec 30, 2023

Hello,
would it be possible to release this ?

@ChristopherRabotin
Copy link
Member

ChristopherRabotin commented Dec 30, 2023 via email

@gwbres
Copy link
Collaborator Author

gwbres commented Dec 30, 2023

Hello Chris,
this won't buy me anything if the other issue is not solved, because my use case is to parse more easily GNSS timestamps and use the Day of Year aswell. So in my case, it can wait for its resolution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristopherRabotin ChristopherRabotin ChristopherRabotin left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gwbres @ChristopherRabotin