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

Empty links should not be errors (warnings only) #367

Conversation

BartlomiejZalas
Copy link
Contributor

@BartlomiejZalas BartlomiejZalas commented Aug 8, 2023

Fixes #276.

  • new TestType URL_EMPTY introduced (with warning severity)
  • MesId EMPTY_LINKS moved from test type URL_INTERNAL to URL_EMPTY
  • Tests adapted to new implementation

Tool usage output:
image

Copy link
Collaborator

@mgautierfr mgautierfr left a comment

Choose a reason for hiding this comment

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

Thanks for your PR.

Just a very small change for cosmetic and we are good.

@@ -70,7 +71,7 @@ std::unordered_map<MsgId, MsgInfo> msgTable = {
{ MsgId::MAIN_PAGE, { TestType::MAIN_PAGE, "Main Page Index stored in Archive Header: {{&main_page_index}}" } },
{ MsgId::EMPTY_ENTRY, { TestType::EMPTY, "Entry {{&path}} is empty" } },
{ MsgId::OUTOFBOUNDS_LINK, { TestType::URL_INTERNAL, "{{&link}} is out of bounds. Article: {{&path}}" } },
{ MsgId::EMPTY_LINKS, { TestType::URL_INTERNAL, "Found {{&count}} empty links in article: {{&path}}" } },
{ MsgId::EMPTY_LINKS, { TestType::URL_EMPTY, "Found {{&count}} empty links in article: {{&path}}" } },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please move this line up or down to regroup the URL_INTERNAL checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved below URL_EXTERNAL

@kelson42
Copy link
Contributor

@BartlomiejZalas Thank you for your PR and last commit. Looks good, but might take a bit more than a week for the next review pass and merge. Thank you for your patience.

@kelson42 kelson42 changed the title empty links should not be errors #276 Empty links should not be errors (warnings only) Aug 12, 2023
@mgautierfr mgautierfr self-requested a review August 21, 2023 12:43
@mgautierfr mgautierfr force-pushed the issue-276/empty-links-should-not-be-errors branch from 133677d to 17b718a Compare August 21, 2023 12:44
@mgautierfr mgautierfr merged commit 16ee85d into openzim:main Aug 21, 2023
3 of 6 checks passed
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.

Empty links should not be errors
3 participants