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 folder structure && CHANGE package-names #1031

Merged

Conversation

ndelangen
Copy link
Member

What I did

I implemented the proposed changes to the folder structure and package names

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

@ndelangen when i try to test it, I get this error:

lerna ERR! execute npm ERR! 404 Not found : @storybook/storybook-database-cloud
lerna ERR! execute npm ERR! 404 
lerna ERR! execute npm ERR! 404  '@storybook/storybook-database-cloud' is not in the npm registry.
lerna ERR! execute npm ERR! 404 You should bug the author to publish it (or use the name yourself!)

It is referenced by addon-comments. I think we should either migrate database-* to the monorepo, revert to @kadira/database-cloud, or remove addon-comments from the monorepo for now.

@@ -13,8 +13,8 @@ Import the `action` function and use it to create actions handlers. When creatin
> *Note: Make sure NOT to use reserved words as function names. [issues#29](https://github.com/storybooks/storybook-addon-actions/issues/29#issuecomment-288274794)*

```js
import { storiesOf, action } from '@storybook/storybook'
// or import { action } from '@storybook/storybook-addon-actions'
import { storiesOf, action } from '@storybook/react'
Copy link
Member

Choose a reason for hiding this comment

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

FYI all of these are going to conflict with the changes I made in #1017

Copy link
Member Author

@ndelangen ndelangen May 14, 2017

Choose a reason for hiding this comment

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

Yup I'll correcting it.

@usulpro
Copy link
Member

usulpro commented May 14, 2017

Thanks, @ndelangen for this rearrangement! 👍

Some points to discuss:

  • rename decorator-centered to addon-centered since this is also an addon. (We don't have special names for panels, storiesOf extensions).

  • move storyshots to addons folder. Since we import it additionally we can call it "addon"

  • changing packages to lib looks good to me, but maybe core also could fit?

  • what is the purpose of the app folder? Maybe it should be located in the examples\ folder?

@ndelangen
Copy link
Member Author

App folder are the what previously were @kadira/storybook and @kadira/storybook-react-native

In the future there may be other UI libraries/frameworks there too.

@ndelangen ndelangen force-pushed the change-packagenames-folderstructure branch from 3c0fde5 to 7d33238 Compare May 14, 2017 20:41
@ndelangen ndelangen force-pushed the change-packagenames-folderstructure branch from 7d33238 to 69b91f6 Compare May 14, 2017 22:08
@codecov
Copy link

codecov bot commented May 14, 2017

Codecov Report

Merging #1031 into am-change-npm-organisation will decrease coverage by 12.57%.
The diff coverage is n/a.

Impacted file tree graph

@@                      Coverage Diff                       @@
##           am-change-npm-organisation   #1031       +/-   ##
==============================================================
- Coverage                       12.57%      0%   -12.58%     
==============================================================
  Files                             200       6      -194     
  Lines                            4478      25     -4453     
  Branches                          714       6      -708     
==============================================================
- Hits                              563       0      -563     
+ Misses                           3283      19     -3264     
+ Partials                          632       6      -626
Impacted Files Coverage Δ
...ges/storyshots/stories/directly_required/Button.js
...ganisation-name/update-organisation-name.output.js
...ackages/react-storybook/demo/src/stories/Button.js
...ts/stories/required_with_context/Button.stories.js
...book-ui/src/modules/ui/components/layout/usplit.js
...orybook-ui/src/modules/ui/containers/left_panel.js
packages/getstorybook/lib/project_types.js
packages/storyshots/src/storybook-channel-mock.js
...s/storybook-ui/src/modules/api/configs/init_api.js
packages/storybook-ui/example/server.js
... and 45 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 b5d2e21...bd95b6e. Read the comment docs.

@ndelangen ndelangen force-pushed the change-packagenames-folderstructure branch from 3132734 to 225c26f Compare May 14, 2017 22:38
@ndelangen ndelangen force-pushed the change-packagenames-folderstructure branch from 225c26f to c1b7e30 Compare May 14, 2017 22:49
@shilman
Copy link
Member

shilman commented May 15, 2017

I agree with @usulpro 's comments above. Otherwise, the changes look good to me. Thanks for all your work on getting this cleaned up!!

@tmeasday
Copy link
Member

Do we want to change storyshots to @storybook/storyshots?

@ndelangen
Copy link
Member Author

I just want to get this finished and merged, so just shout how you want it now, or forever hold your peace 🙊

@ndelangen
Copy link
Member Author

I will continue work on the other branch!

@ndelangen ndelangen merged commit 747dc70 into am-change-npm-organisation May 15, 2017
@ndelangen ndelangen deleted the change-packagenames-folderstructure branch May 15, 2017 10:58
Copy link

nx-cloud bot commented Nov 7, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit bd95b6e. 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
discussion maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants