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

Adds protection against duplicate url #162

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Undid-Iridium
Copy link

Category

This change is exactly one of the following (please change [ ] to [x]) to indicate which:

  • a bug fix (Fix #...)
  • a new Ripper
  • a refactoring
  • a style change/fix
  • a new feature

Description

Instead of keep URL in box, it removes it and issues warnings that the url is already slated to being ripped. Also changed the if condition

Main
idea64_iI2B64mnCz

Branch
java_GaC8668R2V

Testing

Required verification:

  • I've verified that there are no regressions in mvn test (there are no new failures or errors).
  • I've verified that this change works as intended.
    • Downloads all relevant content.
    • Downloads content from multiple pages (as necessary or appropriate).
    • Saves content at reasonable file names (e.g. page titles or content IDs) to help easily browse downloaded content.
  • I've verified that this change did not break existing functionality (especially in the Ripper I modified).

Optional but recommended:

  • I've added a unit test to cover my change.

@soloturn
Copy link

soloturn commented Dec 9, 2023

this does not merge clean, you mind having a look?

@soloturn
Copy link

soloturn commented Dec 9, 2023

@Undid-Iridium merged the other pr and testing fails, have a look on the "actions" tab, e.g.
https://github.com/ripmeapp2/ripme/actions/runs/7151457903/job/19475775478

i created a branch "develop" where all your commits still are. main should be fixed to build and test and thus create a release now. on github server it is headless, so ui test would fail. marked them flaky so they not execute:
https://github.com/ripmeapp2/ripme/actions

@Undid-Iridium
Copy link
Author

Undid-Iridium commented Dec 9, 2023

Sorry - I'll update this to fix that (adding flaky per your suggestion)

@Undid-Iridium
Copy link
Author

Uh, seems to have an issue with another test unit currently. I guess I'll wait till later to retest.

image

@soloturn
Copy link

would you be so kind to squash your commits into one? e.g. on the command line?
https://www.freecodecamp.org/news/git-squash-commits/

@Undid-Iridium Undid-Iridium force-pushed the enhancement_duplicate_url branch 6 times, most recently from 322159a to fd4859e Compare December 30, 2023 19:21
# Conflicts:
#	src/test/java/com/rarchives/ripme/tst/ripper/rippers/ChanRipperTest.java
@Undid-Iridium
Copy link
Author

Unfortunately, squashing after merging main into my branch was a bad idea. I unfortunately put myself in a position where the tree history was too tangled. Rebuilt branch based on main since it took 2 minutes versus the last 30 trying to get the squash to play nice.

@soloturn soloturn force-pushed the main branch 6 times, most recently from 9cdfe51 to bce4ddd Compare March 21, 2024 06:56
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.

2 participants