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

Jump to bottom

update Vagrant file #59

Closed
wants to merge 1 commit into from
Closed

update Vagrant file #59

wants to merge 1 commit into from

Conversation

b4ldr
Copy link
Member

@b4ldr b4ldr commented Sep 27, 2017

This pull updates the Vagrant file as well as the gen_facts scripts to make it easier to genrate facts going forward

@b4ldr b4ldr mentioned this pull request Sep 27, 2017
Closed
@coveralls
Copy link

coveralls commented Sep 27, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 73.529% when pulling 3c0322c on b4ldr:update_vagrant into cb5266f on camptocamp:master.

zachfi
zachfi reviewed Oct 5, 2017
View reviewed changes
facts/get_facts.sh Outdated
done
apt-get install -y make gcc libgmp-dev
;;
'FreeBSD')
pkg update -y
pkg install -y puppet4 rubygem-bundler
Copy link
Contributor

Choose a reason for hiding this comment

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

sysutils/puppet5 is now in the tree, as is sysutils/facter.

@zachfi
Copy link
Contributor

zachfi commented Oct 5, 2017

Any authoritative figures of this repo care to comment on this?

@mcanevet
Copy link
Member

mcanevet commented Oct 6, 2017

@b4ldr why do you replace $(...) with backticks?

@b4ldr
Copy link
Member Author

b4ldr commented Oct 6, 2017 •
edited
Loading

@mcanevet because $(...) is bash specific and freebsd does not have bash

edit: at least thats what i thought but looking at the man pages suggests im wrong, have switched back

@b4ldr
fix conflicts
2e6a40e 
remove facts

switch to puppet5

update
@coveralls
Copy link

coveralls commented Oct 6, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 73.529% when pulling 1722dc5 on b4ldr:update_vagrant into cb5266f on camptocamp:master.

1 similar comment
@coveralls
Copy link

coveralls commented Oct 6, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 73.529% when pulling 1722dc5 on b4ldr:update_vagrant into cb5266f on camptocamp:master.

@coveralls
Copy link

coveralls commented Oct 6, 2017 •
edited
Loading

Coverage Status

Coverage remained the same at 73.529% when pulling 2e6a40e on b4ldr:update_vagrant into cb5266f on camptocamp:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 73.529% when pulling 2e6a40e on b4ldr:update_vagrant into cb5266f on camptocamp:master.

@mcanevet
Copy link
Member

mcanevet commented Oct 7, 2017

There is way too much things in this PR. Could you please at least split it in different explicit commit?

@b4ldr
Copy link
Member Author

b4ldr commented Oct 9, 2017

Really changing two files, the gitignore and adding a simple 6 line script is too big? how would you like me to split this up? TBH i am now spending more time discussing this PR then it took me to make the changes

@mcanevet
Copy link
Member

mcanevet commented Oct 9, 2017

This PR could be easily split into multiple commits. For example, you could create one commit per change in the VagrantFile : "Remove debian-6-x86_64", "Change debian-8-x86_64 box", "Remove ubuntu-10.04-x86_64".
It would be easier for us to revert juste one of your commit in case we need to.

@b4ldr b4ldr closed this Oct 9, 2017
@b4ldr
Copy link
Member Author

b4ldr commented Oct 9, 2017

sorry i dont have the time to rework this PR have close it

@zachfi zachfi mentioned this pull request Oct 10, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zachfi zachfi zachfi left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@b4ldr @coveralls @zachfi @mcanevet