Add support for scoped package (Fix #272) #275

Myhlamaeus · 2017-12-28T20:57:44Z

Fixes #272
Expands upon #273

I’ve needed this feature quite badly, and have thus made the remaining changes myself.

Add tests for repository names with scopes
Add tests for invalid repository names
Composing with this generator should work again
Disambiguate repository name from package name
Fix README.md-URLs for scoped packages

* change option name from `github-account` to `githubAccount` in git/index.js (githubAccount is used everywhere else, and the result is the same) * change option description in readme/index.js to match the description in app/index.js

Add the option `repositoryName`, being the ‘Name of the GitHub repository’.

Fix on node@4.x.x

Myhlamaeus · 2017-12-29T03:59:08Z

I have opened a related PR SBoudrias/inquirer-npm-name#12 to move the validation of the package name to where it belongs.

hemanth · 2017-12-29T04:53:42Z

LGTM

SBoudrias · 2018-01-05T20:46:01Z

generators/git/index.js

+      repository =
+        this.options.githubAccount +
+        '/' +
+        (this.options.repositoryName || this.options.name);


Is there any case where repositoryName would be null and name have a value? I'd prefer we keep a single option here and let the parent generator pass the proper value.

They could be falsy both if some other (non-node) generator would compose with this one or if the user would use it directly through yo node:git.

I’ve just tested this—both githubAccount and repositoryName are marked as required, yet there is no error when omitting them.

I agree with removing this fallback, as this is currently the only position at which name is used, and passing an option as a fallback for the case in which another option is not passed seems absurd.

There are, as I see it, three ways to deal with the possibility of having undefined/undefined as the repository:

Run only git init --quiet when some options are missing

Just ignore it

Prompt for the missing values (which would only work when composing with this generator before prompting)

I’ve implemented the first of the three options for now.

SBoudrias · 2018-01-05T20:47:24Z

generators/app/index.js

+      this.props.repositoryName = this.props.name;
+
+      if (this.props.repositoryName.startsWith('@')) {
+        this.props.repositoryName = this.props.repositoryName


I think here we could also infer the default githubAccount value from the npm scope. (maybe set it as the default of the githubAccount prompt)

If a scoped name is supplied, use the scope part (after the `@` and before the first `/`) as default value for `githubUser`, even ignoring the `authorEmail` which is otherwise used to find the default value.

Only write `repository` to the package file if both `githubAccount` and `repositoryName` are passed to the `git`-generator. Re-read the package before adding the `origin`-remote to repository in case some other generator has modified it.

SBoudrias · 2018-01-21T05:23:58Z

generators/app/index.js

@@ -103,25 +125,38 @@ module.exports = class extends Generator {
    }
  }

+  _initModuleName() {


Let's find a better place for this logic. I don't think the side-effets it has because it's making it hard to know exactly when these values end up being created. In the future, I fear this will be hard to debug.

I have removed the side effects from the method and moved them to _askForModuleName, wherein this.props.name was already being set. _askForModuleName only has side effects, so this should be fine; otherwise, I’d suggest moving the setting of the props out of the _ask*-methods and into the prompting-method.

SBoudrias · 2018-01-21T05:27:06Z

generators/git/index.js

@@ -46,17 +46,21 @@ module.exports = class extends Generator {
    );
  }

-  writing() {
+  _readPkg() {


Why do we need to change this?

If we need to call if before each method, then let's change this to be a local variable rather than a function causing a side-effect on this.

I have removed the side effects and turned it into a local constant.

Have `_getModuleNameParts` return an object with the parts of the module name instead of directly writing it to `props`.

mischah

Thanks so much for this PR ❤️

And Sorry, to kept you hanging in here so long.

mischah · 2018-03-27T20:55:05Z

Relased with v2.4.0 🎉

rodoabad · 2019-08-30T02:58:04Z

@mischah so for scope packages, it's @scope/foo-generator?

wrumsby · 2020-02-27T23:02:39Z

@rodoabad if your package name is @scope/generator-foo then you should be able to install it with yo @scope/foo.

SBoudrias and others added 5 commits December 26, 2017 12:50

Add supports for scoped package (Fix #272)

392633d

Streamline option githubAccount

5633bb6

* change option name from `github-account` to `githubAccount` in git/index.js (githubAccount is used everywhere else, and the result is the same) * change option description in readme/index.js to match the description in app/index.js

Add option repositoryName

79e4d76

Add the option `repositoryName`, being the ‘Name of the GitHub repository’.

fixup! Add supports for scoped package (Fix #272)

66c0e2e

Update package-lock.json

45d7534

Myhlamaeus changed the title ~~Fix/scoped package support~~ Add supports for scoped package (Fix #272) Dec 28, 2017

fixup! fixup! Add supports for scoped package (Fix #272)

6744937

Fix on node@4.x.x

Myhlamaeus changed the title ~~Add supports for scoped package (Fix #272)~~ Add support for scoped package (Fix #272) Dec 28, 2017

Bump dependencies (inquirer-npm-name@3)

f83a224

hemanth requested a review from SBoudrias January 3, 2018 03:59

SBoudrias changed the base branch from fix/scoped-package-support to master January 5, 2018 20:42

SBoudrias reviewed Jan 5, 2018

View reviewed changes

Myhlamaeus added 6 commits January 7, 2018 13:20

Use package scope as default githubUser

c02c98b

If a scoped name is supplied, use the scope part (after the `@` and before the first `/`) as default value for `githubUser`, even ignoring the `authorEmail` which is otherwise used to find the default value.

Drop name-option from git-generator

967d97a

Only write repository into pkg if available

3a08ef9

Only write `repository` to the package file if both `githubAccount` and `repositoryName` are passed to the `git`-generator. Re-read the package before adding the `origin`-remote to repository in case some other generator has modified it.

fixup! Use package scope as default githubUser

98badde

fixup! Only write repository into pkg if available

86631d8

fixup! fixup! Only write repository into pkg if available

065470d

SBoudrias requested changes Jan 21, 2018

View reviewed changes

Myhlamaeus added 2 commits January 30, 2018 03:32

Remove _readPkg side effects from git subgenerator

98599fe

Remove the side effects of _getModuleNameParts

2c410ff

Have `_getModuleNameParts` return an object with the parts of the module name instead of directly writing it to `props`.

mischah approved these changes Mar 27, 2018

View reviewed changes

mischah merged commit 129c233 into yeoman:master Mar 27, 2018

mischah mentioned this pull request Mar 27, 2018

WIP: Add supports for scoped package (Fix #272) #273

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for scoped package (Fix #272) #275

Add support for scoped package (Fix #272) #275

Myhlamaeus commented Dec 28, 2017

Myhlamaeus commented Dec 29, 2017

hemanth commented Dec 29, 2017

SBoudrias Jan 5, 2018

Myhlamaeus Jan 7, 2018 •

edited

Loading

Myhlamaeus Jan 7, 2018

SBoudrias Jan 5, 2018

SBoudrias Jan 21, 2018

Myhlamaeus Jan 30, 2018

SBoudrias Jan 21, 2018

Myhlamaeus Jan 30, 2018

mischah left a comment

mischah commented Mar 27, 2018

rodoabad commented Aug 30, 2019

wrumsby commented Feb 27, 2020

Add support for scoped package (Fix #272) #275

Add support for scoped package (Fix #272) #275

Conversation

Myhlamaeus commented Dec 28, 2017

Myhlamaeus commented Dec 29, 2017

hemanth commented Dec 29, 2017

SBoudrias Jan 5, 2018

Choose a reason for hiding this comment

Myhlamaeus Jan 7, 2018 • edited Loading

Choose a reason for hiding this comment

Myhlamaeus Jan 7, 2018

Choose a reason for hiding this comment

SBoudrias Jan 5, 2018

Choose a reason for hiding this comment

SBoudrias Jan 21, 2018

Choose a reason for hiding this comment

Myhlamaeus Jan 30, 2018

Choose a reason for hiding this comment

SBoudrias Jan 21, 2018

Choose a reason for hiding this comment

Myhlamaeus Jan 30, 2018

Choose a reason for hiding this comment

mischah left a comment

Choose a reason for hiding this comment

mischah commented Mar 27, 2018

rodoabad commented Aug 30, 2019

wrumsby commented Feb 27, 2020

Myhlamaeus Jan 7, 2018 •

edited

Loading