Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

Remove and ignore yarn.lock #1079

Merged
merged 1 commit into from Dec 12, 2019
Merged

Conversation

mturley
Copy link
Contributor

@mturley mturley commented Dec 12, 2019

As discussed in an email thread with @Fryguy and @himdel, we should no longer track yarn.lock on git master.

@mturley mturley added cleanup UI Issues and PRs related only to the UI labels Dec 12, 2019
@mturley mturley added this to In progress in v2v UI via automation Dec 12, 2019
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 7056 lines exceeds the maximum allowed for the inline comments feature.

@miq-bot
Copy link
Member

miq-bot commented Dec 12, 2019

Checked commit mturley@5a9ab94 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

@Fryguy
Copy link
Member

Fryguy commented Dec 12, 2019

Going to merge and see how this goes...we can always revert or rebuild it if necessary

@Fryguy Fryguy merged commit 8a40440 into ManageIQ:master Dec 12, 2019
v2v UI automation moved this from In progress to Done Dec 12, 2019
@Fryguy Fryguy self-assigned this Dec 12, 2019
@Fryguy Fryguy added this to the Sprint 127 Ending Jan 6, 2020 milestone Dec 12, 2019
@mturley mturley deleted the remove-yarn-lock branch December 12, 2019 19:44
@mturley
Copy link
Contributor Author

mturley commented Jan 6, 2020

@Fryguy @himdel we're starting to see in PRs after this one that CI tests are failing just because of changes in our dependencies that are being pulled in because we don't have those versions locked anymore. I'm trying to track them all down and use specific version numbers in package.json for those packages instead of the ^ syntax, but it worries me that our CI can break without any change in our own code (tests fail on master right now). I wonder if we should put yarn.lock back and deal with those CVE warnings another way? I'll see what I can do with pinning versions.

@himdel
Copy link
Contributor

himdel commented Jan 7, 2020

@mturley I think we just need fix the dependencies in package.json to no longer accept versions with breaking changes, I'd assume ~ instead of ^ should do the trick, shouldn't it?

@mturley
Copy link
Contributor Author

mturley commented Jan 7, 2020

That sounds like a good idea, thanks @himdel. I forgot about the ~ operator. Do you think I should go ahead and change all our dependencies to use it now, or wait until we run into any other issues?

@himdel
Copy link
Contributor

himdel commented Jan 8, 2020

Up to you really.. it's always a balancing act between npm audit complaining about too old versions and new versions breaking stuff :), ~ definitely helps, but isn't 100% either.

So, if you'd rather run npm audit regularly (since depedabot won't do it without the lockfile),
and update because you have to, you can get more control that way.

OTOH changing to ~ now means you can mostly stop thinking about it, at least until something new breaks :).

@mturley
Copy link
Contributor Author

mturley commented Jan 8, 2020

I think for now I'll wait for something new to break, but I'll keep this on my mind. Thanks @himdel !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cleanup UI Issues and PRs related only to the UI
Projects
v2v UI
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants