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

Use data from /etc/os-release to determine what branch to pull #62

Closed
wants to merge 1 commit into from
Closed

Use data from /etc/os-release to determine what branch to pull #62

wants to merge 1 commit into from

Conversation

laurelmay
Copy link
Member

The VERSION_CODENAME is how we will be tagging branches in the future.
Source /etc/os-release to get the VERSION_CODENAME variable and pass
that to ansible-pull's --checkout option. Report an error if
/etc/os-release does not exist.

This must not be merged until we create a branch for the Sylvia release.

@laurelmay laurelmay added the do not merge PR is a work in progress and should not be merged label Dec 5, 2017
@laurelmay laurelmay added this to the Spring 2018 milestone Dec 5, 2017
@ripleymj
Copy link
Member

ripleymj commented Dec 5, 2017

Thinking out loud, but could we add a cheat code to this to switch to master branch? Something like if (hostname == "uugmaster") to make ongoing development slightly easier.

@laurelmay
Copy link
Member Author

We could do that. Or have a file like$HOME/.config/vm_config and just source it to see if there's either FORCE_BRANCH=<branch> or USE_MASTER=1.

@ripleymj
Copy link
Member

ripleymj commented Dec 5, 2017

I like FORCE_BRANCH best of the three.

ripleymj
ripleymj approved these changes Dec 6, 2017
View reviewed changes
Copy link
Member

@ripleymj ripleymj left a comment

Choose a reason for hiding this comment

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

I approve of this, but will hold off on merging as we discussed.

@laurelmay laurelmay closed this Dec 12, 2017
@laurelmay laurelmay deleted the prepare-for-release-branches branch December 12, 2017 16:11
@laurelmay laurelmay restored the prepare-for-release-branches branch December 12, 2017 16:11
@laurelmay
Copy link
Member Author

laurelmay commented Dec 12, 2017 •
edited
Loading

Closed by accidentally deleting a branch. Reopening

@laurelmay laurelmay reopened this Dec 12, 2017
ripleymj
ripleymj requested changes Dec 12, 2017
View reviewed changes
Copy link
Member

@ripleymj ripleymj left a comment

Choose a reason for hiding this comment

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

Can't figure out how to revoke my approval, but hoping that by adding "request changes" it will revert. Blocking merge on this pending resolution to the issue mentioned about reading unprivileged data into a root script.

@laurelmay
Copy link
Member Author

@ripleymj The discussed issues hopefully are mostly resolved with this latest commit.

ripleymj
ripleymj requested changes Dec 15, 2017
View reviewed changes
Copy link
Member

@ripleymj ripleymj left a comment

Choose a reason for hiding this comment

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

I need to look closer at this tomorrow, but tentatively: "stip" is not a word, and the temp file should be deleted when done.

@laurelmay
Copy link
Member Author

Those specific concerns should be resolved in the latest force push

ripleymj
ripleymj requested changes Dec 17, 2017
View reviewed changes
Copy link
Member

@ripleymj ripleymj left a comment

Choose a reason for hiding this comment

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

Revoking approval, per your request.

@laurelmay
Copy link
Member Author

I will be pushing a series of commits to test with. I'll advise once this is ready for review again. Pushing to this branch makes testing easier, but unfortunately also updates the pull request

@laurelmay laurelmay force-pushed the prepare-for-release-branches branch 3 times, most recently from c1a56aa to 453b1a1 Compare December 17, 2017 22:30
@laurelmay
Copy link
Member Author

laurelmay commented Dec 17, 2017 •
edited
Loading

3d4ec5b should be ready to be tested and reviewed

@laurelmay
Prepare uug-ansible-wrapper for release branches
3d4ec5b 
The VERSION_CODENAME is how we will be tagging branches in the future.
Source /etc/os-release to get the VERSION_CODENAME variable and pass
that to ansible-pull's --checkout option. Report an error if
/etc/os-release does not exist. Users may also specify their own
configurations in their home configuration directory (defaults to
~/.config/vm_config) to specify a branch and URL
@laurelmay
Copy link
Member Author

Rebased on top of master to resolve merge conflicts

@laurelmay
Copy link
Member Author

@ripleymj don't hate me, but I did a thing: https://gist.github.com/kylelaker/8e8bc9f97514bcda6bfa813d7e7a6f72

Let me know what you think. In a lot of ways, it's better than this wrapper.

@laurelmay laurelmay mentioned this pull request Dec 24, 2017
Merged
@laurelmay laurelmay removed this from the Spring 2018 milestone Dec 26, 2017
@laurelmay
Copy link
Member Author

#94 is way better.

@laurelmay laurelmay closed this Dec 26, 2017
@laurelmay laurelmay deleted the prepare-for-release-branches branch December 27, 2017 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ripleymj ripleymj ripleymj requested changes

Assignees
No one assigned
Labels
do not merge PR is a work in progress and should not be merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@laurelmay @ripleymj