search

department-of-veterans-affairs / veterans-employment-center

Veterans Employment Center
https://www.vets.gov/veterans-employment-center
5 stars 4 forks source link

Pv upgrade rails #377

Closed patrickvinograd closed 7 years ago

patrickvinograd commented 7 years ago

CVEs against 4.2.5.2. Cascading change to schema_plus_index changes schema.rb formatting.

patrickvinograd commented 7 years ago

Fails in bundle-audit due to ruby-saml, which will be removed in subsequent PR #378 which I want to rebase after this is merged.

saneshark commented 7 years ago

@patrickvinograd I noticed that SimpleCov.minimum_coverage is 65, if this gets bumped up to 90, I'm pretty sure test coverage will not be met as a criteria for build passing, but is it worth maybe investigating this further?

I don't have enough back-story to feel 100% comfortable reviewing these changes with test coverage possibly being less than 90%. Happy to review though if its critical we get this through right away. Ping me on slack and lets discuss. Right now it looks like build isn't passing.

ayaleloehr commented 7 years ago

Just chiming in here to add to the record that Patrick and I had talked about this in Slack. Code coverage on this application does not meet our current 90% threshold, but some of this application code will be removed in subsequent PRs and the rest will eventually be ported to vets-api and vets-website, so upping the coverage in this repo is not a priority at this time.

Glad to have these updates in though!