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.

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

Melodic #47

Merged
merged 5 commits into from
Jan 31, 2018
Merged

Melodic #47

merged 5 commits into from
Jan 31, 2018

Conversation

clalancette
Copy link
Contributor

Add the changes to prerelease website for Melodic. I tested it locally, and it seemed to do the correct thing. The static image here is a placeholder until we get the official logo.

@@ -38,6 +38,13 @@ <h1>Select a ROS distribution to Generate a Prerelease Command</h1>
Lunar Loggerhead
</a>
</div>
<div class="col-sm-4 melodic-column text-center">
Copy link
Contributor

Choose a reason for hiding this comment

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

I think, unfortunately, you need to update this one and all the other peers to be col-sm-3, since it's supposed to add up to 12 total. See:

https://getbootstrap.com/docs/3.3/examples/grid/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see. I mistook the col-sm-4 for the number of boxes across, but it is actually the portion of the length across. Makes sense, I'll fix this.

@mikaelarguedas
Copy link
Contributor

Has the prerelease process been tested on bionic ? (just asking, if it's not we may want to defer making it the preferred platform)

@clalancette
Copy link
Contributor Author

Has the prerelease process been tested on bionic ? (just asking, if it's not we may want to defer making it the preferred platform)

Good point. I've tested it out on my Xenial box, where it seems to work fine. I don't currently have a Bionic box that can run docker (I've been doing all my bionic work in containers up to this point), but I can see if there is a live USB boot available for Bionic and try it out.

@clalancette
Copy link
Contributor Author

Good point. I've tested it out on my Xenial box, where it seems to work fine. I don't currently have a Bionic box that can run docker (I've been doing all my bionic work in containers up to this point), but I can see if there is a live USB boot available for Bionic and try it out.

I got Bionic booting on a machine locally, but I can't get docker to run for some reason. I'm not going to spend too much time on this, so I'll just revert the commit changing the preferred platform for now.

@clalancette
Copy link
Contributor Author

All right, all comments addressed in bcb8204 and 82de8af, ready for another review.

Copy link
Contributor

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

lgtm

I got Bionic booting on a machine locally, but I can't get docker to run for some reason. I'm not going to spend too much time on this, so I'll just revert the commit changing the preferred platform for now.

Yeah I think it makes sense to change it only once bionic is out and stable, e.g. we didnt switch the recommended to xenial until a year after the release

@clalancette
Copy link
Contributor Author

Great, thanks for the reviews. Merging.

@clalancette clalancette merged commit dc79dc6 into master Jan 31, 2018
@clalancette clalancette deleted the melodic branch January 31, 2018 18:46
@mikaelarguedas
Copy link
Contributor

Note (as I don't know if it's on purpose): This hasn't been deployed yet

@clalancette
Copy link
Contributor Author

Note (as I don't know if it's on purpose): This hasn't been deployed yet

Not really on purpose, I didn't get to it yet and I don't know how to do it :). Will bug someone today about getting it deployed.

@clalancette
Copy link
Contributor Author

Deployed now, thanks to @tfoote .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants