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

Use dependencies.io Lerna support & disable issue creation #1803

Closed
wants to merge 0 commits into from
Closed

Conversation

Hypnosphi
Copy link
Member

@codecov
Copy link

codecov bot commented Sep 6, 2017

Codecov Report

Merging #1803 into release/3.3 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           release/3.3    #1803   +/-   ##
============================================
  Coverage        22.24%   22.24%           
============================================
  Files              325      325           
  Lines             6522     6522           
  Branches           831      832    +1     
============================================
  Hits              1451     1451           
+ Misses            4448     4425   -23     
- Partials           623      646   +23
Impacted Files Coverage Δ
app/vue/src/server/utils.js 0% <0%> (-53.58%) ⬇️
app/react/src/client/preview/error_display.js 0% <0%> (ø) ⬆️
addons/storyshots/src/test-bodies.js 19.14% <0%> (ø) ⬆️
addons/info/src/components/types/ArrayOf.js 14.28% <0%> (ø) ⬆️
lib/ui/src/modules/shortcuts/actions/shortcuts.js 22.41% <0%> (ø) ⬆️
addons/knobs/src/components/types/Color.js 8.1% <0%> (ø) ⬆️
lib/ui/src/modules/ui/routes.js 0% <0%> (ø) ⬆️
addons/info/src/components/types/ObjectType.js 14.28% <0%> (ø) ⬆️
...react-native/src/manager/components/PreviewHelp.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/ui/libs/filters.js 47.36% <0%> (ø) ⬆️
... and 28 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 a268c50...3a6b832. Read the comment docs.

@ndelangen
Copy link
Member

ndelangen commented Sep 6, 2017

why based on 3.3 ?.
I think we should base this on master.

@Hypnosphi
Copy link
Member Author

master doesn't have yarn.lock

@Hypnosphi
Copy link
Member Author

After a discussion in slack, I disabled issue creators:

hypnos [2:52 AM] 
Do we want issues for major updates in deps? https://github.com/Hypnosphi/storybook/issues
 

norbert 
[2:56 AM] 
what do you mean?


hypnos [3:06 AM] 
The recommended setup for `dependencies.io` is PRs for minors and patches, issues for majors (edited)


shilman 
[3:28 AM] 
sounds a little heavy-handed to me, but that’s just personal preference


[3:28] 
i wouldn’t do issues if it was up to me, but i am also open to being pursuaded (edited)


dduan
[5:47 PM] 
I think it just creates unnecessary spam and nothing actionable. Perhaps a few people want to own upgrading dependencies and be assigned those issues? We have enough open tickets already.

@Hypnosphi Hypnosphi changed the title Use dependencies.io Lerna support Use dependencies.io Lerna support & disable issue creation Sep 8, 2017
dependencies.yml Outdated
- type: js-lerna
path: /
settings:
bootstrap_command: yarn && yarn bootstrap -- --core
Copy link
Member

Choose a reason for hiding this comment

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

I think we want to hold this until #1810

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I'll add both to 3.3 milestone (see #1803 (comment))

Copy link
Member Author

Choose a reason for hiding this comment

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

In fact, the command has to stay the same: the bootstrap script won't work without root depandencies

@ndelangen ndelangen added maintenance User-facing maintenance tasks dependencies labels Sep 8, 2017
@Hypnosphi Hypnosphi added this to the v3.3.0 milestone Sep 8, 2017
@ndelangen
Copy link
Member

Ok, just to get everything clear:

  • Merging this would initially do nothing, since the tool would only trigger on master right?
  • This would replace greenkeeper completely, after this lands on master, we should disable greenkeeper.

@Hypnosphi
Copy link
Member Author

Both true

@Hypnosphi
Copy link
Member Author

@ndelangen
Copy link
Member

ndelangen commented Sep 28, 2017

I think the best way to proceed with this, is to get yarn-workspaces and yarn lock files on master and get this merged to master.

What do you think @Hypnosphi @shilman @igor-dv @storybooks/team ?


Greenkeeper has already been disabled for the entire organisation

@Hypnosphi
Copy link
Member Author

I will do that as long as everyone is OK with it

@danielduan
Copy link
Member

seems reasonable to me. This is a developer oriented change so merging to master wouldn't really affect the product itself.

@ndelangen
Copy link
Member

@Hypnosphi Can you make the PR? I think it's a go.

@Hypnosphi
Copy link
Member Author

Should it be one PR for yarn and then another one for dependencies.io?

@ndelangen
Copy link
Member

Let's do yarn & workspaces first, and then dependencies.io after

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

Successfully merging this pull request may close these issues.

3 participants