Issue 29720577: Issue 6288 - Publish stylelint-config-eyeo on npm (Closed)
|
Created:
March 12, 2018, 9:24 a.m. by ire Modified:
April 13, 2018, 9:18 a.m. Base URL:
https://hg.adblockplus.org/codingtools Visibility:
Public. |
DescriptionIssue 6288 - Publish stylelint-config-eyeo on npm
Patch Set 1 #Patch Set 2 : Rebase #Patch Set 3 : Update version number #MessagesTotal messages: 23
Tristan/Sebastian could you please take a look at this? According to this npm documentation, in order to create a package scoped to an organisation, I need to update the package name to include the organisation name. See https://www.npmjs.com/docs/orgs/publishing-an-org-scoped-package.html Note: The only change I made here was in stylelint-config-eyeo/package.json. There seems to be an issue with previous commits not being updated to the master bookmark. When that is sorted out I will rebase so those other changes will not be part of this review.
On 2018/03/12 09:30:27, ire wrote: > Tristan/Sebastian could you please take a look at this? > > According to this npm documentation, in order to create a package scoped to an > organisation, I need to update the package name to include the organisation > name. See https://www.npmjs.com/docs/orgs/publishing-an-org-scoped-package.html > > Note: The only change I made here was in stylelint-config-eyeo/package.json. > There seems to be an issue with previous commits not being updated to the master > bookmark. When that is sorted out I will rebase so those other changes will not > be part of this review. The "master"-branch not being synced should be fixed by now. "eyeo" @ npm already has one package published, see https://www.npmjs.com/org/eyeo However, the relevant package (eslint-config-eyeo in codingtools) does not '@eyeo/' in it's name-parameter in package.json. Are we sure that this is the way to go? How was it done for eslint-config-eyeo?
On 2018/03/12 09:43:59, tlucas wrote: > The "master"-branch not being synced should be fixed by now. Thank you! > "eyeo" @ npm already has one package published, see > https://www.npmjs.com/org/eyeo > > However, the relevant package (eslint-config-eyeo in codingtools) does not > '@eyeo/' in it's name-parameter in package.json. > > Are we sure that this is the way to go? How was it done for eslint-config-eyeo? I asked about this because I noticed the same thing. Peter, who did the transfer, said he didn't actually do anything to transfer the package to the organisation. He said that as soon as he changed the eyeo user to an organisation, it happened automatically. I've been searching for other ways to do this because I also think that this doesn't seem like the only way to create a package under an organisation. However, the only documentation related to this that I could find from npm specified this particular way.
When I published eslint-config-eyeo eyeo was a user and not an organisation. The ops team have since changed eyeo to be an organisation. To be honest I'm not exactly sure how to publish under an organisation, or if there's anything different we have to do now! Personally for this I would follow the documentation, get the change reviewed, then attempt to publish to npm, if it works push the change to codingtools but otherwise update the review and attempt to publish again.
On 2018/03/12 10:07:14, kzar1 wrote: > When I published eslint-config-eyeo eyeo was a user and not an organisation. The > ops team have since changed eyeo to be an organisation. To be honest I'm not > exactly sure how to publish under an organisation, or if there's anything > different we have to do now! > > Personally for this I would follow the documentation, get the change reviewed, > then attempt to publish to npm, if it works push the change to codingtools but > otherwise update the review and attempt to publish again. Thanks for your advice. In that case, this looks almost good to me - as soon as you tried publishing it in the proposed way :)
On 2018/03/12 10:10:51, tlucas wrote: > On 2018/03/12 10:07:14, kzar1 wrote: > > When I published eslint-config-eyeo eyeo was a user and not an organisation. > The > > ops team have since changed eyeo to be an organisation. To be honest I'm not > > exactly sure how to publish under an organisation, or if there's anything > > different we have to do now! > > > > Personally for this I would follow the documentation, get the change reviewed, > > then attempt to publish to npm, if it works push the change to codingtools but > > otherwise update the review and attempt to publish again. > > Thanks for your advice. > > In that case, this looks almost good to me - as soon as you tried publishing it > in the proposed way :) Alright then, thanks!
Update: This worked, but the name of the package was actually `@eyeo/stylelint-config-eyeo`, instead of just `stylelint-config-eyeo`. This isn't necessarily a huge problem, we could change the documentation to reflect this. But it still seems like there should be a way around this. Looking at other orgs (e.g. bcc https://www.npmjs.com/org/bbc), there should be a way to not have the organisation prefix. I unpublished the package for now, and I want to continue searching around a bit more. I stumbled upon this issue on npm (https://github.com/npm/npm/issues/16150) which coincidentally outlined a process for making a package part of an organisation. I'm not sure if that will work, but I want to try it. However, I have to wait 24 hours to republish the package because I originally published (then unpublished) the package just under my name. So, I will try again tomorrow.
Another Update: I believe that the way to do this is to publish the package under my name then grant the organisation access to it (see https://docs.npmjs.com/misc/orgs and https://github.com/npm/npm/issues/16150). I successfully published the package (although I had to increase the version number because of a prior conflict with me unpublishing the package) - https://www.npmjs.com/package/stylelint-config-eyeo I attempted to grant access to the eyeo developers team by running the following command: ``` npm access grant read-write eyeo:developers ``` However, this command failed with 403 presumably because I don't have permission to add the eyeo team to this package. See this description - https://docs.npmjs.com/misc/orgs#description So, would I be able to get temporary Team admin privileges in order to run this command? I don't see any other way around it since I have already published the package under my own account.
tluca kzar: Please see my previous message
On 2018/04/10 15:48:45, ire wrote: > tluca kzar: Please see my previous message Hey - i'm sorry, but i think neither Dave nor me can help here (i bet none of us has the permissions to grant admin rights on npmjs.org) I guess the most promising approach would be to file a hub-ticket for granting you temporary permissions. Cheers!
On 2018/04/10 16:25:46, tlucas wrote: > On 2018/04/10 15:48:45, ire wrote: > > tluca kzar: Please see my previous message > > Hey - i'm sorry, but i think neither Dave nor me can help here (i bet none of us > has the permissions to grant admin rights on http://npmjs.org) > I guess the most promising approach would be to file a hub-ticket for granting > you temporary permissions. > > Cheers! Alright, thanks!
This has been done now, I didn't need to make this change to the repo, so closing this review. Thanks all!
On 2018/04/12 14:38:08, ire wrote: > This has been done now, I didn't need to make this change to the repo, so > closing this review. Thanks all! I spoke too soon :/ I forgot that I had to update the version number in the package.json file. Please review.
On 2018/04/10 16:25:46, tlucas wrote: > Hey - i'm sorry, but i think neither Dave nor me can help here (i bet none of us > has the permissions to grant admin rights on http://npmjs.org) > I guess the most promising approach would be to file a hub-ticket for granting > you temporary permissions. Well, at least Dave published the eslint-config-eyeo package on npmjs. So I would assume him would have the credentials. (I'm not sure how exactly npmjs works, but FWIW it would be great if the respective module owner/peers for codingtools would be able to publish updates to our npm packages from this repository.)
On 2018/04/12 14:52:13, Sebastian Noack wrote: > On 2018/04/10 16:25:46, tlucas wrote: > > Hey - i'm sorry, but i think neither Dave nor me can help here (i bet none of > us > > has the permissions to grant admin rights on http://npmjs.org) > > I guess the most promising approach would be to file a hub-ticket for granting > > you temporary permissions. > > Well, at least Dave published the eslint-config-eyeo package on npmjs. So I > would assume him would have the credentials. (I'm not sure how exactly npmjs > works, but FWIW it would be great if the respective module owner/peers for > codingtools would be able to publish updates to our npm packages from this > repository.) Yes currently all the developers under the eyeo organisation have access to do that. I believe if more are added to the organisation they will also have access.
On 2018/04/12 14:56:35, ire wrote: > On 2018/04/12 14:52:13, Sebastian Noack wrote: > > On 2018/04/10 16:25:46, tlucas wrote: > > > Hey - i'm sorry, but i think neither Dave nor me can help here (i bet none > of > > us > > > has the permissions to grant admin rights on http://npmjs.org) > > > I guess the most promising approach would be to file a hub-ticket for > granting > > > you temporary permissions. > > > > Well, at least Dave published the eslint-config-eyeo package on npmjs. So I > > would assume him would have the credentials. (I'm not sure how exactly npmjs > > works, but FWIW it would be great if the respective module owner/peers for > > codingtools would be able to publish updates to our npm packages from this > > repository.) > > Yes currently all the developers under the eyeo organisation have access to do > that. I believe if more are added to the organisation they will also have > access. Besides the npmjs.org topic, it seems your patch sets are not complete - could you upload one patch set, containing all the changes since master? That way we only have to apply 1 diff (rietveld should handle this just fine) - thank you :)
On 2018/04/12 15:12:41, tlucas wrote: > On 2018/04/12 14:56:35, ire wrote: > > On 2018/04/12 14:52:13, Sebastian Noack wrote: > > > On 2018/04/10 16:25:46, tlucas wrote: > > > > Hey - i'm sorry, but i think neither Dave nor me can help here (i bet none > > of > > > us > > > > has the permissions to grant admin rights on http://npmjs.org) > > > > I guess the most promising approach would be to file a hub-ticket for > > granting > > > > you temporary permissions. > > > > > > Well, at least Dave published the eslint-config-eyeo package on npmjs. So I > > > would assume him would have the credentials. (I'm not sure how exactly npmjs > > > works, but FWIW it would be great if the respective module owner/peers for > > > codingtools would be able to publish updates to our npm packages from this > > > repository.) > > > > Yes currently all the developers under the eyeo organisation have access to do > > that. I believe if more are added to the organisation they will also have > > access. > > Besides the http://npmjs.org topic, it seems your patch sets are not complete - could > you upload one patch set, containing all the changes since master? > That way we only have to apply 1 diff (rietveld should handle this just fine) - > thank you :) I don't understand what you mean. Isn't Patch Set 3 enough? This (https://codereview.adblockplus.org/29720577/diff/29750611/stylelint-config-ey...) is the only change I have made since master.
On 2018/04/12 15:15:56, ire wrote: > On 2018/04/12 15:12:41, tlucas wrote: > > On 2018/04/12 14:56:35, ire wrote: > > > On 2018/04/12 14:52:13, Sebastian Noack wrote: > > > > On 2018/04/10 16:25:46, tlucas wrote: > > > > > Hey - i'm sorry, but i think neither Dave nor me can help here (i bet > none > > > of > > > > us > > > > > has the permissions to grant admin rights on http://npmjs.org) > > > > > I guess the most promising approach would be to file a hub-ticket for > > > granting > > > > > you temporary permissions. > > > > > > > > Well, at least Dave published the eslint-config-eyeo package on npmjs. So > I > > > > would assume him would have the credentials. (I'm not sure how exactly > npmjs > > > > works, but FWIW it would be great if the respective module owner/peers for > > > > codingtools would be able to publish updates to our npm packages from this > > > > repository.) > > > > > > Yes currently all the developers under the eyeo organisation have access to > do > > > that. I believe if more are added to the organisation they will also have > > > access. > > > > Besides the http://npmjs.org topic, it seems your patch sets are not complete > - could > > you upload one patch set, containing all the changes since master? > > That way we only have to apply 1 diff (rietveld should handle this just fine) > - > > thank you :) > > I don't understand what you mean. Isn't Patch Set 3 enough? This > (https://codereview.adblockplus.org/29720577/diff/29750611/stylelint-config-ey...) > is the only change I have made since master. Ah sorry - i did not realize that patch set 1 wasn't rebased yet. However, the change from patch-set 2 is not included in patch set 3 (i.e. - "name": "stylelint-config-eyeo", + "name": "@eyeo/stylelint-config-eyeo", ) -> is that intentionally? I gotta admit, i kind of lost track here.
On 2018/04/12 15:38:50, tlucas wrote: > On 2018/04/12 15:15:56, ire wrote: > > On 2018/04/12 15:12:41, tlucas wrote: > > > On 2018/04/12 14:56:35, ire wrote: > > > > On 2018/04/12 14:52:13, Sebastian Noack wrote: > > > > > On 2018/04/10 16:25:46, tlucas wrote: > > > > > > Hey - i'm sorry, but i think neither Dave nor me can help here (i bet > > none > > > > of > > > > > us > > > > > > has the permissions to grant admin rights on http://npmjs.org) > > > > > > I guess the most promising approach would be to file a hub-ticket for > > > > granting > > > > > > you temporary permissions. > > > > > > > > > > Well, at least Dave published the eslint-config-eyeo package on npmjs. > So > > I > > > > > would assume him would have the credentials. (I'm not sure how exactly > > npmjs > > > > > works, but FWIW it would be great if the respective module owner/peers > for > > > > > codingtools would be able to publish updates to our npm packages from > this > > > > > repository.) > > > > > > > > Yes currently all the developers under the eyeo organisation have access > to > > do > > > > that. I believe if more are added to the organisation they will also have > > > > access. > > > > > > Besides the http://npmjs.org topic, it seems your patch sets are not > complete > > - could > > > you upload one patch set, containing all the changes since master? > > > That way we only have to apply 1 diff (rietveld should handle this just > fine) > > - > > > thank you :) > > > > I don't understand what you mean. Isn't Patch Set 3 enough? This > > > (https://codereview.adblockplus.org/29720577/diff/29750611/stylelint-config-ey...) > > is the only change I have made since master. > > Ah sorry - i did not realize that patch set 1 wasn't rebased yet. > > However, the change from patch-set 2 is not included in patch set 3 > (i.e. > > - "name": "stylelint-config-eyeo", > + "name": "@eyeo/stylelint-config-eyeo", > > ) > > -> is that intentionally? I gotta admit, i kind of lost track here. No worries, yes that is intentional. The change in Patch Set 3 is all that is needed here.
> No worries, yes that is intentional. The change in Patch Set 3 is all that is > needed here. Alright, sorry for the confusion. In that case it's LGTM
On 2018/04/12 15:46:30, tlucas wrote: > > No worries, yes that is intentional. The change in Patch Set 3 is all that is > > needed here. > > Alright, sorry for the confusion. In that case it's LGTM Thanks! I will email you the exported patch now. 
|
