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

Bump mastarm to v5.0.2 to resolve security vulnerabilities #442

Merged
merged 6 commits into from
Jul 17, 2019
Merged

Conversation

landonreed
Copy link
Contributor

@landonreed landonreed commented Jun 20, 2019

Checklist

  • Appropriate branch selected (all PRs must first be merged to dev before they can be merged to master)
  • Any modified or new methods or classes have helpful JSDoc and code is thoroughly commented
  • The description lists all applicable issues this PR seeks to resolve
  • The description lists any configuration setting(s) that differ from the default settings
  • All tests and CI builds passing

Description

Bump mastarm to 5.0.2 to fix vulnerabilities.

@landonreed
Copy link
Contributor Author

It looks like there are a lot of new flow type issues under mastarm 5.

@evansiroky
Copy link
Contributor

What level of effort would it take to fix the flow-type errors? I think we ought to revert the flow upgrade in mastarm if it's too much effort.

@evansiroky evansiroky assigned landonreed and unassigned evansiroky Jun 27, 2019
@landonreed
Copy link
Contributor Author

@evansiroky, I'm not exactly sure, but basically all of the actions are broken (including some stuff in flow-typed/redux-actions). There's a chance that some fix in the flow-typed repo could fix this stuff up, but I haven't really dug in. Let me spend some time with this to see if I can resolve stuff quickly.

@landonreed
Copy link
Contributor Author

Ah, looks like this was due to a change in 0.85 https://medium.com/flow-type/asking-for-required-annotations-64d4f9c1edf8

@landonreed
Copy link
Contributor Author

@evansiroky, I'm not quick enough with absorbing this flow stuff to make it worth our time, so I would say we should bump down to v0.85.0 of flow-bin in mastarm unless you want to take a crack.

@landonreed landonreed assigned evansiroky and unassigned landonreed Jul 1, 2019
@evansiroky
Copy link
Contributor

Yeah, let's definitely downgrade flow-bin in mastarm to the latest possible version that still works, but also create an issue to update to the latest flow.

@landonreed
Copy link
Contributor Author

OK, I've got a branch ready to push once I get the correct permissions set by Trevor on mastarm.

@evansiroky
Copy link
Contributor

OK, I've got a branch ready to push once I get the correct permissions set by Trevor on mastarm.

I just made you an admin on mastarm.

landonreed added a commit to conveyal/mastarm that referenced this pull request Jul 1, 2019
0.85.0 introduces a host of new flow type issues in Data Tools. This is explained in this blog post:
https://medium.com/flow-type/asking-for-required-annotations-64d4f9c1edf8

re ibi-group/datatools-ui#442
@evansiroky
Copy link
Contributor

Assigning back to @landonreed to update to latest mastarm.

@evansiroky evansiroky assigned landonreed and unassigned evansiroky Jul 2, 2019
@codecov-io
Copy link

codecov-io commented Jul 2, 2019

Codecov Report

Merging #442 into dev will increase coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #442      +/-   ##
==========================================
+ Coverage   15.09%   15.09%   +<.01%     
==========================================
  Files         325      325              
  Lines       15830    15833       +3     
  Branches     4803     4804       +1     
==========================================
+ Hits         2389     2390       +1     
- Misses      11496    11498       +2     
  Partials     1945     1945
Impacted Files Coverage Δ
lib/editor/util/validation.js 42.55% <ø> (ø) ⬆️
lib/public/components/UserAccount.js 0% <0%> (ø) ⬆️
lib/gtfsplus/components/GtfsPlusField.js 0% <0%> (ø) ⬆️
lib/editor/util/index.js 30.76% <0%> (+1.28%) ⬆️

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 3b7ed18...526e761. Read the comment docs.

@landonreed landonreed assigned evansiroky and unassigned landonreed Jul 17, 2019
@landonreed
Copy link
Contributor Author

@evansiroky, I think you mentioned that you might already have fixes for the flow issues here. Is that correct?

@evansiroky
Copy link
Contributor

Yep: a9dec1f

@evansiroky
Copy link
Contributor

Assigning back to Landon so the Travis builds can run properly.

@evansiroky evansiroky removed their assignment Jul 17, 2019
@landonreed landonreed changed the title Bump mastarm to v5.0.1 to resolve security vulnerabilities Bump mastarm to v5.0.2 to resolve security vulnerabilities Jul 17, 2019
@landonreed landonreed assigned evansiroky and unassigned landonreed Jul 17, 2019
@landonreed
Copy link
Contributor Author

OK, build is complete. Once you add a review, I think we can merge.

@evansiroky evansiroky assigned landonreed and unassigned evansiroky Jul 17, 2019
@landonreed landonreed merged commit 5f95dd3 into dev Jul 17, 2019
@landonreed landonreed deleted the mastarm5 branch July 17, 2019 19:51
@landonreed
Copy link
Contributor Author

🎉 This PR is included in version 4.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants