-
Notifications
You must be signed in to change notification settings - Fork 71
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
restructure content fetching from github #465
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh, I’m on my phone and the review interface got a little weird, hope the comments made it through fine.
bin/get_all_the_diffs
Outdated
begin | ||
response = open("https://raw.githubusercontent.com/voxpupuli/#{repo}/master/.sync.yml") | ||
rescue OpenURI::HTTPError | ||
puts "something is fucked up with #{repo} and https://raw.githubusercontent.com/voxpupuli/#{repo}/master/.sync.yml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove vulgarity from error messages. Some may appreciate it but it’s not fully inclusive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry. removed it
bin/get_all_the_diffs
Outdated
response = open("https://raw.githubusercontent.com/voxpupuli/#{repo}/master/.sync.yml") | ||
rescue OpenURI::HTTPError | ||
puts "something is fucked up with #{repo} and https://raw.githubusercontent.com/voxpupuli/#{repo}/master/.sync.yml" | ||
modules_that_have_missing_secrets << repo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a plan for this variable? I want to say it should be named more accurately (missing_sync?) but I don’t see it used anywhere at all.
bin/get_all_the_diffs
Outdated
puts "We need to add Puppets version_requirement to: #{modules_without_puppet_version_range.join(', ')}" if modules_without_puppet_version_range.count > 0 | ||
puts "We need to adjust the list of operatingsystemrelease for Debian to #{DEBIAN_SUPPORT_RANGE} on: #{supports_wrong_debian_version.join(', ')}" | ||
puts "We need to adjust the list of operatingsystemrelease for Ubuntu to #{UBUNTU_SUPPORT_RANGE} on: #{supports_wrong_ubuntu_version.join(', ')}" | ||
puts "We need to adjust the list of operatingsystemrelease for CentOS to #{CENTOS_SUPPORT_RANGE} on: #{supports_wrong_centos_version.join(', ')}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these have if guards as well?
bin/get_all_the_diffs
Outdated
begin | ||
response = open("https://raw.githubusercontent.com/voxpupuli/#{repo}/master/.msync.yml") | ||
rescue OpenURI::HTTPError | ||
puts "something is fucked up with #{repo} and https://raw.githubusercontent.com/voxpupuli/#{repo}/master/.msync.yml" | ||
puts "something is fucked up with #{repo} and https://raw.githubusercontent.com/voxpupuli/#{repo}/master/.msync.yml" if DEBUG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the vulgarity here, please? It’s going to be funny to some and off putting to others.
bin/get_all_the_diffs
Outdated
begin | ||
response = open("https://raw.githubusercontent.com/voxpupuli/#{repo}/master/.msync.yml") | ||
rescue OpenURI::HTTPError | ||
puts "something is fucked up with #{repo} and https://raw.githubusercontent.com/voxpupuli/#{repo}/master/.msync.yml" | ||
puts "something is fucked up with #{repo} and https://raw.githubusercontent.com/voxpupuli/#{repo}/master/.msync.yml" if DEBUG | ||
modules_that_were_added_but_never_synced << repo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not seem to be used
bin/get_all_the_diffs
Outdated
response = open("https://raw.githubusercontent.com/voxpupuli/#{repo}/master/.sync.yml") | ||
rescue OpenURI::HTTPError | ||
puts "something is fucked up with #{repo} and https://raw.githubusercontent.com/voxpupuli/#{repo}/master/.sync.yml" if DEBUG | ||
modules_that_have_missing_secrets << repo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not seem to be used
@bastelfreak what about the operating system release sections, will they spit out junk if there’s no issues? |
@rnelson0 I fixed that |
It looks like you're done but still has WIP in the title, let me know if you're good to go or if I should revoke my review. |
nope, all good! |
No description provided.