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

fix: do not check the same url more than once #2301

Closed

Conversation

peterprototypes
Copy link
Contributor

This is a fix for #2298

Before:

DeepinScreenshot_select-area_20230912162708

After:

DeepinScreenshot_select-area_20230912162728

I've used the https://github.com/organicmaps/organicmaps.github.io/ repo for the before and after tests.

IMPORTANT: Please do not create a Pull Request adding a new feature without discussing it first.

The place to discuss new features is the forum: https://zola.discourse.group/
If you want to add a new feature, please open a thread there first in the feature requests section.

Sanity check:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Code changes

(Delete or ignore this section for documentation changes)

  • Are you doing the PR on the next branch?

If the change is a new feature or adding to/changing an existing one:

  • Have you created/updated the relevant documentation page(s)?

components/site/src/link_checking.rs Outdated Show resolved Hide resolved
@peterprototypes
Copy link
Contributor Author

@Keats looks like me and @biodranik worked on the same thing. I think his solution is a bit easier to follow, as it avoids the complexity of a nested HashMap. The only difference in end result is that I changed the output to show the number of unique external links instead of the total number of links.

@biodranik
Copy link
Contributor

Printing the real number of unique urls that are checked is a good idea.

In my approach failures are also printed for each page where the same link was used, so you can see how often the failed link is used, and on which pages. Is it the same for your implementation?

@peterprototypes
Copy link
Contributor Author

Printing the real number of unique urls that are checked is a good idea.

In my approach failures are also printed for each page where the same link was used, so you can see how often the failed link is used, and on which pages. Is it the same for your implementation?

Yes, it's the same. I also clone the result from the first checked link to all messages.

@biodranik
Copy link
Contributor

Learning Rust in this way is a lot of fun! I've also updated my PR #2305, @Keats WDYT?

@Keats
Copy link
Collaborator

Keats commented Sep 13, 2023

I think #2305 approach is a bit neater

@peterprototypes
Copy link
Contributor Author

I think #2305 approach is a bit neater

Agreed 👍

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