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

Adjust to number of captures in previous regex #360

Merged
merged 5 commits into from
Jul 31, 2023
Merged

Adjust to number of captures in previous regex #360

merged 5 commits into from
Jul 31, 2023

Conversation

mgautierfr
Copy link
Collaborator

supersedes #359

@codecov
Copy link

codecov bot commented Jul 17, 2023 •
edited
Loading

Codecov Report

Patch coverage: 28.57% and project coverage change: +0.17% 🎉

Comparison is base (82109ba) 27.52% compared to head (e4dc68e) 27.70%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #360      +/-   ##
==========================================
+ Coverage   27.52%   27.70%   +0.17%     
==========================================
  Files          26       26              
  Lines        2521     2527       +6     
  Branches     1346     1349       +3     
==========================================
+ Hits          694      700       +6     
+ Misses       1356     1346      -10     
- Partials      471      481      +10
Files Changed Coverage Δ
src/zimwriterfs/zimcreatorfs.h 16.66% <ø> (ø)
src/zimwriterfs/zimcreatorfs.cpp 22.77% <28.57%> (+2.36%) ⬆️

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

@mgautierfr mgautierfr mentioned this pull request Jul 17, 2023
Closed
@kelson42 kelson42 added this to the 3.3.0 milestone Jul 17, 2023
veloman-yunkan
veloman-yunkan requested changes Jul 22, 2023
View reviewed changes
test/zimwriterfs-zimcreatorfs.cpp Outdated Show resolved Hide resolved
test/zimwriterfs-zimcreatorfs.cpp Outdated Show resolved Hide resolved
test/zimwriterfs-zimcreatorfs.cpp Outdated Show resolved Hide resolved
haoess and others added 4 commits July 28, 2023 15:52
@haoess @mgautierfr
Adjust to number of captures in previous regex
3379db0
@mgautierfr
Add test on parsing of redirect file.
413eca8 
- Refactor code to move parsing code in `parse_redirectArticles`.
- Add test.
@mgautierfr
Add gtest wrap in subproject.
7262d3e 
This way, zim-tools is properly tested even if gtest is not available
in the system.
(This is what is done in libzim and libkiwix)
@mgautierfr
fixup! Add test on parsing of redirect file.
2302871
veloman-yunkan
veloman-yunkan requested changes Jul 28, 2023
View reviewed changes
Copy link
Collaborator

@veloman-yunkan veloman-yunkan left a comment

Choose a reason for hiding this comment

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

LGTM.

Please rebase&fixup&approve&merge

test/zimwriterfs-zimcreatorfs.cpp Outdated Show resolved Hide resolved
@mgautierfr
Add test with too many tabs and fix code.
e4dc68e 
`(.+)` is a bit to greedy and matches `\t`.
@mgautierfr
Copy link
Collaborator Author

@veloman-yunkan comment fixed.
Packages CI failing is not related to this change.

I'm merging.

@mgautierfr mgautierfr merged commit 662a593 into main Jul 31, 2023
9 of 13 checks passed
@mgautierfr mgautierfr deleted the fix_redirect_regex branch July 31, 2023 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@veloman-yunkan veloman-yunkan veloman-yunkan requested changes

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

Successfully merging this pull request may close these issues.
4 participants
@mgautierfr @veloman-yunkan @kelson42 @haoess