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

change NPM organisation from kadira to storybook in code #996

Merged
merged 19 commits into from
May 16, 2017

Conversation

aaronmcadam
Copy link
Contributor

@aaronmcadam aaronmcadam commented May 6, 2017

Issue: #773

Blocked by

#1004, #1008

What I did

Created the codemod

How to test

Run unit tests

TODO

  • Confirm new package names
  • remove storybook- prefixes
  • getstorybook/generators
  • LICENSEs
  • CODE_OF_CONDUCTs
  • README badge URLs
  • react-storybook/src/server/track_usage.js

New Package directories and names

Current Repo Path Suggested Repo Path Publish Name
packages/getstorybook packages/cli @storybook/cli
packages/react-storybook packages/react @storybook/react
packages/react-native-storybook packages/react-native @storybook/react-native
packages/storybook-ui packages/ui @storybook/ui
packages/channels @storybook/channels
packages/channel-websocket packages/channels-websocket @storybook/channels-websocket
packages/addons @storybook/addons
packages/addon-actions packages/addons-actions @storybook/addons-actions
packages/decorator-centered packages/decorators-centered @storybook/decorators-centered
packages/storyshots @storybook/storyshots

@codecov
Copy link

codecov bot commented May 6, 2017

Codecov Report

Merging #996 into master will increase coverage by 0.05%.
The diff coverage is 6.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #996      +/-   ##
==========================================
+ Coverage   12.51%   12.57%   +0.05%     
==========================================
  Files         196      200       +4     
  Lines        4458     4478      +20     
  Branches      713      714       +1     
==========================================
+ Hits          558      563       +5     
- Misses       3269     3283      +14     
- Partials      631      632       +1
Impacted Files Coverage Δ
packages/addon-comments/src/manager/index.js 0% <ø> (ø) ⬆️
packages/react-storybook/src/server/track_usage.js 0% <ø> (ø) ⬆️
packages/addon-options/src/manager/index.js 0% <ø> (ø) ⬆️
packages/addon-actions/src/index.js 0% <ø> (ø) ⬆️
packages/addon-actions/src/manager.js 0% <ø> (ø) ⬆️
packages/getstorybook/lib/detect.js 0% <ø> (ø) ⬆️
...toryshots/stories/required_with_context/Welcome.js 0% <ø> (ø) ⬆️
packages/addon-knobs/src/register.js 0% <0%> (ø) ⬆️
...ts/stories/required_with_context/Button.stories.js 0% <0%> (ø) ⬆️
...ents/src/manager/containers/CommentsPanel/index.js 0% <0%> (ø) ⬆️
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 42219f5...bcbc570. Read the comment docs.

@aaronmcadam aaronmcadam changed the title feat: add codemod for migrating NPM organisation refactor: change NPM organisation from kadira to storybook May 6, 2017
@aaronmcadam aaronmcadam force-pushed the am-change-npm-organisation branch 2 times, most recently from 2adbf62 to 3a7e455 Compare May 6, 2017 10:10
@aaronmcadam
Copy link
Contributor Author

@ndelangen Can we confirm what we want the published name these packages to be? Should we drop all storybook- prefixes?

@aaronmcadam aaronmcadam requested a review from ndelangen May 7, 2017 23:29
Copy link
Member

@ndelangen ndelangen left a comment

Choose a reason for hiding this comment

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

Do we need to add the codemod as a package? I'd think not.

@ndelangen
Copy link
Member

So I've been trying this, but it would appear that in order to get this to work, all packages must at least have been published once on npm, in order for lerna to be able to bootstrap.

@aaronmcadam
Copy link
Contributor Author

aaronmcadam commented May 8, 2017 via email

@aaronmcadam
Copy link
Contributor Author

Lerna doesn't need the packages to be published to bootstrap. Tests will fail though where they reference the new package names.

@ndelangen
Copy link
Member

ndelangen commented May 8, 2017

Lerna doesn't need the packages to be published to bootstrap

That's what I'm hearing from lerna maintainers, however check the travis logs, bootstrap does fail...

It would be useful for users to migrate

Right, I'd move them somewhere else though, maybe ./migrations. I feel like we should reserve ./packages/ for functional parts of storybook.

@ndelangen
Copy link
Member

I'd personally like to remove the storybook- prefix, what you you think?

@aaronmcadam
Copy link
Contributor Author

@ndelangen I think we should only have storybook mentioned in the name for the main package.

So the directory structure would go from this:

addon-actions
addon-comments
addon-graphql
addon-info
addon-knobs
addon-notes
addon-options
addons
channel
channel-postmessage
channel-websocket
codemod
decorator-centered
getstorybook
react-native-storybook
react-storybook
storybook-ui
storyshots

to this:

addon-actions
addon-comments
addon-graphql
addon-info
addon-knobs
addon-notes
addon-options
addons
channel
channel-postmessage
channel-websocket
codemod
decorator-centered
getstorybook
native-storybook
storybook
ui
storyshots

@aaronmcadam
Copy link
Contributor Author

aaronmcadam commented May 9, 2017

@ndelangen regarding the codemod, I understand where you're coming from regarding keeping packages for functionality.

If we move it elsewhere, we'll have to manually manage publishing it.

We might be able to make it work by setting the packages config in lerna.json to take an extra tools directory.

@usulpro
Copy link
Member

usulpro commented May 9, 2017

Hi!
Isn't storybook-ui more comfortable than ui as an important part of the core?

And is it clear that native-storybook is related to react-native?

@ndelangen
Copy link
Member

@ndelangen regarding the codemod, I understand where you're coming from regarding keeping packages for functionality.

If we move it elsewhere, we'll have to manually manage publishing it.

We might be able to make it work by setting the packages config in lerna.json to take an extra tools directory.

Good suggestion! I'm already using the packages config. It's real easy to add a tools folder.

@ndelangen
Copy link
Member

Isn't storybook-ui more comfortable than ui as an important part of the core?

Do you prefer @storybook/ui or @storybook/storybook-ui?

@theinterned
Copy link
Member

I agree that @storybook/native-storybook is more confusing then @storybook/react-native-storybook ... the name of the project is react-native so arbitrarily cutting it in half seems odd.

@aaronmcadam
Copy link
Contributor Author

I was trying to aim for parity between the directory names on disk and the package names on npm. So @kadira/react-storybook has gone to @storybooks/storybook. I do agree that the name for the native project is awkward.

@ndelangen ndelangen added this to the v3.0.0 milestone May 10, 2017
@aaronmcadam
Copy link
Contributor Author

aaronmcadam commented May 10, 2017

Ok, so the latest consensus is:

addon-actions
addon-comments
addon-graphql
addon-info
addon-knobs
addon-notes
addon-options
addons
channel
channel-postmessage
channel-websocket
codemod
decorator-centered
getstorybook
react
react-native
(storybook-ui|ui)
storyshots

@ndelangen
Copy link
Member

ndelangen commented May 10, 2017

Sorry to through my 2ct in the mix again... Here's my suggestion:

Current Repo Path Suggested Repo Path Publish Name
packages/getstorybook packages/cli @storybook/cli
packages/react-storybook packages/react @storybook/react
packages/react-native-storybook packages/react-native @storybook/react-native
packages/storybook-ui packages/ui @storybook/ui
packages/channels @storybook/channels
packages/channel-websocket packages/channels-websocket @storybook/channels-websocket
packages/addons @storybook/addons
packages/addon-actions packages/addons-actions @storybook/addons-actions
packages/decorator-centered packages/decorators-centered @storybook/decorators-centered
packages/storyshots @storybook/storyshots

@aaronmcadam
Copy link
Contributor Author

To all those watching this PR, I've added the name changes to the PR description.

@usulpro
Copy link
Member

usulpro commented May 10, 2017

Okey
but why @storybook/addons-actions and @storybook/decorators-centered ? (originally it was without s)

And wouldn't it better to rename @storybook/decorators-centered to @storybook/addon-centered since decorator is just a kind of addon?

@aaronmcadam
Copy link
Contributor Author

@usulpro I think we should have the same prefix for these namespaced packages so that it's easier to see that packages are related visually.

I don't mind moving decorators-centered into the addons namespace.

@usulpro usulpro added the maintenance User-facing maintenance tasks label May 12, 2017
@usulpro
Copy link
Member

usulpro commented May 12, 2017

How do you like the idea (if it still possible) to move all addons into a separate folder storybook/addons and remove addon from folder name:
storybook/packages/addon-actions -> storybook/addons/actions

# Conflicts:
#	examples/cra-storybook/package.json
#	examples/cra-storybook/src/stories/index.js
#	examples/test-cra/package.json
#	examples/test-cra/src/stories/index.js
#	packages/addon-actions/.storybook/addons.js
#	packages/addon-comments/.storybook/addons.js
#	packages/addon-comments/package.json
#	packages/addon-info/.storybook/addons.js
#	packages/addon-info/example/story.js
#	packages/addon-info/package.json
#	packages/addon-knobs/.storybook/addons.js
#	packages/addon-knobs/README.md
#	packages/addon-notes/README.md
#	packages/addon-notes/package.json
#	packages/addon-options/.storybook/addons.js
#	packages/getstorybook/generators/METEOR/template/.stories/index.js
#	packages/getstorybook/generators/REACT/template/stories/index.js
#	packages/getstorybook/generators/REACT_NATIVE/template/storybook/addons.js
#	packages/getstorybook/generators/REACT_NATIVE/template/storybook/stories/index.js
#	packages/getstorybook/generators/REACT_SCRIPTS/template/src/stories/index.js
#	packages/react-native-storybook/docs/manual-setup.md
#	packages/react-native-storybook/package.json
#	packages/react-native-storybook/src/index.js
#	packages/react-native-storybook/src/server/addons.js
#	packages/react-storybook/demo/package.json
#	packages/react-storybook/demo/src/stories/index.js
#	packages/react-storybook/src/client/index.js
#	packages/react-storybook/src/server/addons.js
#	packages/storyshots/package.json
#	packages/storyshots/stories/required_with_context/Button.stories.js
#	packages/storyshots/stories/required_with_context/Welcome.stories.js
…ructure

CHANGE folder structure && CHANGE package-names
@ndelangen ndelangen merged commit a9944e1 into master May 16, 2017
shilman added a commit to shilman/storybook that referenced this pull request May 17, 2017
@ajhyndman ajhyndman mentioned this pull request May 17, 2017
ndelangen pushed a commit that referenced this pull request May 24, 2017
@ndelangen ndelangen changed the title refactor: change NPM organisation from kadira to storybook change NPM organisation from kadira to storybook in code May 27, 2017
@Hypnosphi Hypnosphi deleted the am-change-npm-organisation branch August 17, 2017 22:28
Copy link

nx-cloud bot commented Jul 23, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit bcbc570. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants