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.

Sign up for GitHub

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

Switch $releasevar to ${::os[release][major]} #577

Merged
merged 1 commit into from
Aug 30, 2017
+26 −14
Merged

Switch $releasevar to ${::os[release][major]} #577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion manifests/repo/rhel.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Class: rabbitmq::repo::rhel
# Makes sure that the Packagecloud repo is installed
class rabbitmq::repo::rhel(
$location = 'https://packagecloud.io/rabbitmq/rabbitmq-server/el/$releasever/$basearch',
$location = "https://packagecloud.io/rabbitmq/rabbitmq-server/el/${::os[release][major]}/\$basearch",
Copy link
Member

Choose a reason for hiding this comment

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

👍 for structured facts

Copy link
Member

Choose a reason for hiding this comment

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

Bonus points if you also use the $facts hash?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexjfisher should have thought of that... too bad it's already merged, but feel free to submit a PR 😉

$key_source = 'https://www.rabbitmq.com/rabbitmq-release-signing-key.asc',
) {

Expand Down
2 changes: 1 addition & 1 deletion spec/classes/rabbitmq_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@
end
it 'the repo should be present, and contain the expected values' do
is_expected.to contain_yumrepo('rabbitmq').with(ensure: 'present',
baseurl: 'https://packagecloud.io/rabbitmq/rabbitmq-server/el/$releasever/$basearch',
baseurl: 'https://packagecloud.io/rabbitmq/rabbitmq-server/el/7/$basearch',
gpgkey: 'https://www.rabbitmq.com/rabbitmq-release-signing-key.asc')
end
end
Expand Down
36 changes: 24 additions & 12 deletions spec/spec_helper_local.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@

def with_debian_facts
let :facts do
super().merge(operatingsystemmajrelease: '6',
lsbdistcodename: 'squeeze',
lsbdistid: 'Debian',
osfamily: 'Debian',
os: {
name: 'Debian',
release: { full: '6.0' }
})
super().merge(
operatingsystemmajrelease: '6',
lsbdistcodename: 'squeeze',
lsbdistid: 'Debian',
osfamily: 'Debian',
os:
{
name: 'Debian',
release: { full: '6.0' }
}
)
end
end

Expand All @@ -28,15 +31,24 @@ def with_openbsd_facts
# operatingsystemrelease may contain X.X-current
# or other prefixes
let :facts do
super().merge(kernelversion: '5.9',
osfamily: 'OpenBSD')
super().merge(
kernelversion: '5.9',
osfamily: 'OpenBSD'
)
end
end

def with_redhat_facts
let :facts do
super().merge(operatingsystemmajrelease: '7',
osfamily: 'Redhat')
super().merge(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this what you had in mind? I think the previous layout may have just been what rubocop -a did, or else copying the example of what was there before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed one more update, let me know if it looks right. It didn't look right when I indented the curly brace after the newline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As you can probalby see, I also updated the Debian / OpenBSD facts.

But agree that we should rework this whole thing at some point.

operatingsystemmajrelease: '7',
osfamily: 'Redhat',
os:
{
name: 'CentOS',
release: { major: '7' }
}
)
end
end

Expand Down