Conversation
terraflubb
left a comment
terraflubb
left a comment
There was a problem hiding this comment.
Looks good, one suggestion which isn't really a suggestion, just a thing I like to draw attention to when anybody does it, but in this case it doesn't really matter.
🧇 🐳
| if ::Gem::VERSION =~ /^[012]\./ | ||
| command << "--no-rdoc" << "--no-ri" << resource[:name] | ||
| else | ||
| # Rubygems 3.0.0 changed --no-ri to --no-document | ||
| command << "--no-document" << resource[:name] | ||
| end |
There was a problem hiding this comment.
It's not important enough to request it as a change, but I wanted to flag it since it's a tiiiiny example of something I see a lot in stpst code (not you specifically, just in general, even by me when I miss it)
When a conditional has a common component, yanking it out sets you up for success. Because if that thing grows, it becomes more duplication to maintain. Or if it changes in one it might be forgotten in the other.
Again, this is like 10 characters so it doesn't matter here at all, I'm not that pedantic.
| if ::Gem::VERSION =~ /^[012]\./ | |
| command << "--no-rdoc" << "--no-ri" << resource[:name] | |
| else | |
| # Rubygems 3.0.0 changed --no-ri to --no-document | |
| command << "--no-document" << resource[:name] | |
| end | |
| if ::Gem::VERSION =~ /^[012]\./ | |
| command << "--no-rdoc" << "--no-ri" | |
| else | |
| # Rubygems 3.0.0 changed --no-ri to --no-document | |
| command << "--no-document" | |
| end | |
| command << resource[:name] |
There was a problem hiding this comment.
oh yeah I agree, but I didn't touch that part of the code 😄 That's made by puppetlabs
3.8.7.stpst.10...nedap:puppet-gem:stpst11 is what I changed here
| # REVISIT: This is less that ideal, I think, but right now I am more | ||
| # comfortable watching us ship with some copyright than without any; we | ||
| # can redress that when it becomes appropriate. --daniel 2011-04-27 | ||
| it :copyright do should =~ /20\d{2}/ end |
There was a problem hiding this comment.
Weird that the syntax changed! The possessive its sounds better to me when dealing with properties. But better to be consistent, I guess. 👍
2 participants
Ref nedap/puppet/pull/16172
Diff to stpst10, not main, to see what actually changed. 3.8.7.stpst.10...nedap:puppet-gem:stpst11
We only use this repo with the tags, not on main/master
Ref nedap/steppingstone/issues/10588