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

doc: update with recommended integration into package.json #281

Merged
merged 27 commits into from Dec 2, 2019

Conversation

mhdawson
Copy link
Member

@mhdawson mhdawson commented Nov 9, 2019

Fixes: #241 (comment)

Update to add recommended integration with package.json
as outlined in: #241 (comment)

Fixes: nodejs#241 (comment)

Update to add recommended integration with package.json
as outlined in: nodejs#241 (comment)
@mhdawson
Copy link
Member Author

mhdawson commented Nov 9, 2019

@ljharb, @dominykas, @nodejs/package-maintenance

@mhdawson
Copy link
Member Author

mhdawson commented Nov 9, 2019

Also @Eomm

docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
[repository](https://docs.npmjs.com/files/package.json#repository) entry. In this case the filapath for the file containing the support information will be
`./package-support.json`
* a string: a relative path, starting with ./, that points to a file within the
repository specified in the [repository](https://docs.npmjs.com/files/package.json#repository)
Copy link
Member

Choose a reason for hiding this comment

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

(it'd be nicer to review if these weren't hard-wrapped)

docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
entry which contains the canonical support information.
* object, with repository key matching the schema for the existing
[repository](https://docs.npmjs.com/files/package.json#repository) entry, and optional
path field that comnplies with the format described above under `a string`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
path field that comnplies with the format described above under `a string`
`path` field that complies with the format described above under `a string`

docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
“no external internet” use cases as best as can be achieved.

In the future, we hope that the developer experience could be further inproved
through itegration with the npm client. For validation could be integrated into
Copy link
Member

Choose a reason for hiding this comment

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

Should we open a draft RFC (in both - yarn and npm - possibly other managers?) so that we can link to it?

Copy link
Member Author

Choose a reason for hiding this comment

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

That sounds reasonable to me, although I don't think we need to block this PR on that. We can add the links once we open the issues.

docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Outdated Show resolved Hide resolved
docs/drafts/PACKAGE-SUPPORT.md Show resolved Hide resolved
mhdawson and others added 15 commits November 10, 2019 11:54
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
Co-Authored-By: Dominykas Blyžė <hello@dominykas.com>
Co-Authored-By: Dominykas Blyžė <hello@dominykas.com>
Co-Authored-By: Dominykas Blyžė <hello@dominykas.com>
Co-Authored-By: Dominykas Blyžė <hello@dominykas.com>
Co-Authored-By: Dominykas Blyžė <hello@dominykas.com>
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
mhdawson and others added 6 commits November 12, 2019 02:55
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
Co-Authored-By: Vincent Weevers <mail@vincentweevers.nl>
Co-Authored-By: Jordan Harband <ljharb@gmail.com>
@mhdawson
Copy link
Member Author

@ljharb, @vweevers comments addressed.

@mhdawson
Copy link
Member Author

@nodejs/package-maintenance we'll need a few more reviewers to get to the 4 needed to land.

This recommendation is designed to meet the following
conflicting requirements:
* ability to review and process the support information
off-line with access to only the package itself.
Copy link
Member

Choose a reason for hiding this comment

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

can we remove the hard-wrapping throughout the doc? it's easier to review and suggest when arbitrary newlines aren't inserted all over the place :-)

Copy link
Member

Choose a reason for hiding this comment

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

@johnmuhl why the thumbs down?

Copy link
Member

Choose a reason for hiding this comment

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

This is a subjective thing, which I am guessing causes the thumbs down. Depending on your editor and configuration, these newlines can make this document legible where otherwise it would not be. In this particular line it might not matter, but I know sometimes when I have vim open on a wide screen, a long single line paragraph is painful to read. Just my 2c.

EDIT: I am not saying it should be one way or another, I just wanted to point out it is heavily subjective.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, but the GitHub UI always wraps by default, and suggestions are single line, and vim/editors can easily be configured to soft wrap if desired.

@nodejs nodejs deleted a comment from dominykas Nov 18, 2019
mhdawson and others added 5 commits November 19, 2019 15:45
Co-Authored-By: John Muhl <johnmuhl@users.noreply.github.com>
Co-Authored-By: John Muhl <johnmuhl@users.noreply.github.com>
Co-Authored-By: John Muhl <johnmuhl@users.noreply.github.com>
Co-Authored-By: John Muhl <johnmuhl@users.noreply.github.com>
Co-Authored-By: John Muhl <johnmuhl@users.noreply.github.com>
@mhdawson
Copy link
Member Author

Ok we have 4th reviewer but need 7 days to pass so will wait to land until then.

@mhdawson
Copy link
Member Author

mhdawson commented Dec 2, 2019

7 Days have passed, landing !

@mhdawson mhdawson merged commit daeb7ea into nodejs:master Dec 2, 2019
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.

Future direction support field
7 participants