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

NameMapper detects (and reports) all naming conflicts #1067

Merged
merged 7 commits into from
Mar 6, 2024

Conversation

veloman-yunkan
Copy link
Collaborator

This PR demonstrates and addresses some issues with name mapping that were discovered while working on kiwix/kiwix-tools#596.

Copy link

codecov bot commented Mar 5, 2024

Codecov Report

Attention: Patch coverage is 45.45455% with 6 lines in your changes missing coverage. Please review.

Project coverage is 41.05%. Comparing base (ddde6db) to head (068555d).
Report is 85 commits behind head on main.

Files with missing lines Patch % Lines
src/name_mapper.cpp 45.45% 0 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1067      +/-   ##
==========================================
+ Coverage   41.03%   41.05%   +0.01%     
==========================================
  Files          58       58              
  Lines        4187     4190       +3     
  Branches     2303     2304       +1     
==========================================
+ Hits         1718     1720       +2     
  Misses       1003     1003              
- Partials     1466     1467       +1     

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

Two entries in test library.xml used to point to the same file path.
Note that though the third entry pointed to a different file name
it is a symbolic link to the same file.

Now all three entries point to pseudo-different files (having the
same content behind them).

This change is needed so that tests don't break when detection of
conflicting book names is made stricter.
The extended setup of the NameMapper unit test demonstrates (by the fact
that this change doesn't break the tests that check the stderr) that
certain naming conflicts escape NameMapper's attention.
No cleanup of the new function was performed to keep the diff minimal.
- Got rid of the continue statement
- Renamed the function parameter
- Fixed indentation
Also this change leads to the change in the mapping (since conflicts
that previously went undetected and just overwrote the existing entry
are now rejected).
@mgautierfr mgautierfr merged commit a8368b3 into main Mar 6, 2024
16 checks passed
@mgautierfr mgautierfr deleted the stricter_namemapper branch March 6, 2024 13:24
@kelson42 kelson42 added this to the 13.2.0 milestone Mar 7, 2024
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.

3 participants