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

Use https when possible. #7

Merged
merged 1 commit into from
Jul 23, 2018
Merged

Use https when possible. #7

merged 1 commit into from
Jul 23, 2018

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR requested a review from a team as a code owner July 11, 2018 06:38
@XhmikosR
Copy link
Contributor Author

So, this needs the node-semver PR first merged, released and the lib updated here for tests to pass. Let me know how to proceed.

@XhmikosR
Copy link
Contributor Author

PR rebased and removed the node-semver changes; when the package is updated any changes will be picked up.

@@ -2580,7 +2580,7 @@ Yep. Pretty sure. Maybe. Hmm... I hope.

*Sighs audibly.*

[Let us know](http://github.com/npm/npm/issues/new) if we broke something else
[Let us know](https://github.com/npm/npm/issues/new) if we broke something else
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably but not related to the https change :P

Copy link
Contributor

@karlhorky karlhorky Jul 19, 2018

Choose a reason for hiding this comment

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

heh, looks like both the link to npm/npm issues and the link to npm/cli issues are actually broken.

Looks like npm/cli moved their issues to Discourse. Maybe the #support or #bugs categories would be appropriate here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the old repo has been archived...

I can fix this one, np.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are thousands of old links that could be updated. The commit ones should be safe to do since the hashes should be the same. PR/issue links should stay. And changelog.js needs to be updated.

@@ -5023,7 +5023,7 @@ not just made modules using it difficult to deploy.
Again, this is a _**BETA RELEASE**_, so not everything is working just yet.
Here are the issues that we already know about. If you run into something
that isn't on this list,
[let us know](https://github.com/npm/npm/issues/new)!
[let us know](https://github.com/npm/cli/issues/new)!
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, looks like this is a broken link also, because npm/cli moved their issues to Discourse.

Maybe the #support or #bugs categories would be appropriate here.

cc @zkat

Copy link
Contributor

Choose a reason for hiding this comment

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

No need to modify this file at all. Please leave this as-is for historical purposes.

Copy link
Contributor

@zkat zkat left a comment

Choose a reason for hiding this comment

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

I appreciate the time taken to change links to https, but I have some requests for this particular one:

I don't think we should be changing most of these URLs at all. I've gone through and marked the ones that I think are worth keeping, but I think the others should be reverted.

Usually, when I get a big diff like this, I prefer making sure the diff is actually adding significant value. Just doing a sweeping search-and-replace is relatively low-value when you look at the actual URLs, and in general, I'd much prefer to keep certain things intact.

@@ -259,7 +259,7 @@ Happy hacking!
### v2.15.8 (2016-06-17):

There's a very important bug fix and a long-awaited (and significant!)
deprecation in this hotfix release. [Hold on.](http://butt.holdings/)
deprecation in this hotfix release. [Hold on.](https://butt.holdings/)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather not modify archived changelogs.

@@ -865,7 +865,7 @@ unrecoverable errors then npm would crash instead of printing the error.
### v3.10.1 (2016-06-17):

There are two very important bug fixes and one long-awaited (and significant!)
deprecation in this hotfix release. [Hold on.](http://butt.holdings/)
deprecation in this hotfix release. [Hold on.](https://butt.holdings/)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto this changelog. Please leave this as-is.

@@ -5023,7 +5023,7 @@ not just made modules using it difficult to deploy.
Again, this is a _**BETA RELEASE**_, so not everything is working just yet.
Here are the issues that we already know about. If you run into something
that isn't on this list,
[let us know](https://github.com/npm/npm/issues/new)!
[let us know](https://github.com/npm/cli/issues/new)!
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to modify this file at all. Please leave this as-is for historical purposes.

@@ -248,7 +248,7 @@ Also there's maybe a bit of an easter egg in this release. 'Cause those are fun
* [`2f3e4b6`](https://github.com/npm/npm/commit/2f3e4b645cdc268889cf95ba24b2aae572d722ad)
[#15833](https://github.com/npm/npm/pull/15833)
Mention the [24-hour unpublish
policy](http://blog.npmjs.org/post/141905368000/changes-to-npms-unpublish-policy)
policy](https://blog.npmjs.org/post/141905368000/changes-to-npms-unpublish-policy)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto for not modifying changelogs.

@@ -496,7 +496,7 @@ up your CI work by **2-5x** in some cases! Have a good example? Tell us on
Twitter!

`npm ci` is, right now, [the fastest
installer](http://blog.npmjs.org/post/171556855892/introducing-npm-ci-for-faster-more-reliable)
installer](https://blog.npmjs.org/post/171556855892/introducing-npm-ci-for-faster-more-reliable)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto about changelogs.

@@ -608,7 +608,7 @@ Trying to install another plugin with a conflicting requirement will cause an
error. For this reason, make sure your plugin requirement is as broad as
possible, and not to lock it down to specific patch versions.

Assuming the host complies with [semver](http://semver.org/), only changes in
Assuming the host complies with [semver](https://semver.org/), only changes in
Copy link
Contributor

Choose a reason for hiding this comment

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

This is ok.

@@ -102,8 +102,8 @@ here to help.**

If you think another npm publisher is infringing your trademark, such as by
using a confusingly similar package name, email <[email protected]> with a link to
the package or user account on [https://npmjs.com](https://npmjs.com). Attach a
copy of your trademark registration certificate.
the package or user account on [https://www.npmjs.com/](https://www.npmjs.com/).
Copy link
Contributor

Choose a reason for hiding this comment

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

Kinda weird but it's fine.

@@ -81,7 +81,7 @@ effectively implement the entire CouchDB API anyway.

## Is there a website or something to see package docs and such?

Yes, head over to <https://npmjs.com/>
Yes, head over to <https://www.npmjs.com/>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is ok.

@@ -56,7 +56,7 @@
</head>
<h1>npm</h1>

<p>npm is a package manager for <a href="http://nodejs.org/">node</a>. You can use it to install
<p>npm is a package manager for <a href="https://nodejs.org/">node</a>. You can use it to install
Copy link
Contributor

Choose a reason for hiding this comment

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

Also ok.

@@ -8,7 +8,7 @@
# shell living at /bin/sh.
#
# See this helpful document on writing portable shell scripts:
# http://www.gnu.org/s/hello/manual/autoconf/Portable-Shell.html
# https://www.gnu.org/s/hello/manual/autoconf/Portable-Shell.html
Copy link
Contributor

Choose a reason for hiding this comment

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

Also ok.

@XhmikosR
Copy link
Contributor Author

@zkat please check again.

@zkat zkat changed the base branch from latest to release-next July 23, 2018 20:49
@zkat zkat removed the in-progress label Jul 23, 2018
@zkat zkat merged commit e115f9d into npm:release-next Jul 23, 2018
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.

4 participants