Skip to content

Conversation

@NullPiotrException
Copy link

Ran this command in qBittorrent repo directory:
grep -rnw . --exclude=\*.{svg,} -e "http:"

Then checked all URLs if they support HTTPS now. If they did, I changed "http://" to "https://" in the URL.
Sometimes I was redirected to another page. For such cases I modified the entire URL.
Sometimes got 404. Checked Internet Archive and if there was a non-404 snapshot, I changed the URL to Web Archive version from the latest working snapshot.
I also tried googling the snapshot to check if there's a new version that's just not been redirected to. If there was, I changed the URL to a working page instead of the archive.

E.g.: URL: https://www.dyndns.com/developers/specs
Web Archive version: https://web.archive.org/web/20110716154636/https://www.dyndns.com/developers/specs
Googled and got: https://help.dyn.com/remote-access-api/

This archive on the other hand I just changed to another source (Wikipedia) as it contains the same info (and more) as the old URL.

Copy link
Member

@sledgehammer999 sledgehammer999 left a comment

Choose a reason for hiding this comment

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

Partial review. Changes in the following files should be reverted:

  1. qtsingleapplication module: It is a 3rd party module and we update it from to time to time
  2. The *.m4 scripts. They are 3rd party and we update them from time to time.
  3. Links in the GPL license text. IIRC those are part of the license and shouldn't be changed.
  4. The various *.js* files. Again 3rd party.
  5. The configure script is autogenerated.
  6. Scripts under build-aux folder. 3rd party.

(wait for others to confirm)

@Chocobo1
Copy link
Member

Chocobo1 commented May 6, 2018

Changes in the following files should be reverted:

I agree with @sledgehammer999.

@Chocobo1
Copy link
Member

@sledgehammer999
Close? It doesn't look like OP is coming back....

And maybe we should close other abandoned PRs too, with "abandoned" tagged.

@NullPiotrException
Copy link
Author

I am coming back. I wanted to fix URLs in mentioned projects first. I already maked a PR for qt-solutions (creators of qtsingleapplication) but it appears that they don't allow Github PRs so I'll probably have to create an account on some other platform.

But I disagree with all the points, tbh. No offence.
Ad. 1, 2, 4, 6. Even if there's a 3rd party module/lib, you can still modify it. Of course it means that if a new version comes, all changes will be losed. But it's still not a problem to have better/working URLs at least for a little while and me making another commit to break them again is just a waste of time. I don't see any downside of changing them for now.
Ad. 3. I can't imagine that a license will be breaked if you change URL from http to https. What if there's a link in the license that's not working but has an updated version? Imo it shouldn't be a problem to change it, especially if the project is FOSS which makes it very improbable that the creators will take legal actions because of such a thing.
Ad. 5. Gtk, I'll find the script that generates it and fix that. But still, nothing wrong with changing it for now.

But you're the boss, @sledgehammer999. I'll try to fix the URLs in the 3rd party libs and then get back to you here. I'd prefer if you didn't close this PR yet (bigger motivation for me), but go for it if you prefer to have a clear PR section.

@sledgehammer999
Copy link
Member

But you're the boss, @sledgehammer999. I'll try to fix the URLs in the 3rd party libs and then get back to you here. I'd prefer if you didn't close this PR yet (bigger motivation for me), but go for it if you prefer to have a clear PR section.

Let's take the middle ground if you please.

  1. I'll close this
  2. You can make a new PR with points 1-6 taken into account so that part can be merged ASAP.
  3. Once 3rd party modules/libs/etc have updated their URLs, we can make new PRs to update them here too.

Thanks for the effort in finding all these.

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