Skip to content

updates gems#88

Merged
techgique merged 4 commits into
devfrom
gem_update
Nov 20, 2018
Merged

updates gems#88
techgique merged 4 commits into
devfrom
gem_update

Conversation

@jduss4
Copy link
Copy Markdown
Contributor

@jduss4 jduss4 commented Nov 12, 2018

loofah updated to 2.2.3 https://nvd.nist.gov/vuln/detail/CVE-2018-16468
bumped the version of the repo as well to reflect the patch, but then kinda discovered that we don't have a good place to put the version at this point, it was just hacked into the config file and that's not a sustainable way to do it. I'll open an issue.

Copy link
Copy Markdown
Member

@techgique techgique left a comment

Choose a reason for hiding this comment

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

Looks good, but we can probably tackle the versioning issue here yet too, yes?

@jduss4
Copy link
Copy Markdown
Contributor Author

jduss4 commented Nov 15, 2018

@techgique how does this solution look? I could either add it straight in there or relative require a version.rb file or something?

https://stackoverflow.com/a/11359471/4154134

@jduss4
Copy link
Copy Markdown
Contributor Author

jduss4 commented Nov 15, 2018

I could just change this line here, really, to not pull from a config file at all: https://github.com/CDRH/api/blob/dev/config/initializers/config.rb#L5

@techgique
Copy link
Copy Markdown
Member

techgique commented Nov 19, 2018

I think I like the config/initializers/version.rb approach from the Stack Overflow link. I like that it gives version changes their own file named version. And then assign from Api::Application::VERSION in that config initializer.

Looking for version changes on the initializer file config.rb in git history could be confusing and read like we're changing how the app itself is configured rather than updating the app's version.

@techgique techgique merged commit 184b08c into dev Nov 20, 2018
@techgique techgique deleted the gem_update branch November 20, 2018 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants