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

improving requirements for updates in the development guide #3

Merged
merged 3 commits into from
Jun 20, 2017

Conversation

chargio
Copy link
Contributor

@chargio chargio commented Jun 12, 2017

This PR changes some details to reflect updates in the development guide:

  • Makes memory for the VM 6 GB, instead of 4
  • Install yarn
  • Improves the method used to reboot the VM when needed
  • Updates hostname to avoid "-" and "_", just in case.

@chargio chargio requested a review from Fryguy June 12, 2017 13:11
@chargio chargio self-assigned this Jun 12, 2017
@Fryguy
Copy link
Member

Fryguy commented Jun 13, 2017

Updates ruby to 2.4.1

ManageIQ appliances nor docker containers do this, so why is this here? The project doesn't even work on 2.4 yet.

@Fryguy
Copy link
Member

Fryguy commented Jun 13, 2017

@simaishi Please review

Vagrantfile Outdated
echo "and do $ bin/setup to finish configuration"
echo "Server can be started with $ bundle exec rake evm:start"
echo "Server can be started with $ bin/rails evm:start"
Copy link
Member

@Fryguy Fryguy Jun 13, 2017

Choose a reason for hiding this comment

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

I don't think we recommend bin/rails over bundle exec rake anywhere else in our guides for other platforms. You can see the same even in the guides link you have below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will change it back to bundle exec then

- bower
- yarn
- gulp-cli
- webpack
Copy link
Member

@Fryguy Fryguy Jun 13, 2017

Choose a reason for hiding this comment

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

I don't think you need webpack here nor gulp-cli, but I could be wrong. @simaishi?

Choose a reason for hiding this comment

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

right, packages needed are just bower and yarn as per ui build kickstart partial

Copy link
Contributor Author

@chargio chargio Jun 14, 2017

Choose a reason for hiding this comment

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

I updated it from the developers guide, that should be corrected if we are not asking for this.

http://www.manageiq.org/docs/guides/developer_setup

Choose a reason for hiding this comment

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

Maybe it's needed for "developer setup" then?? I'm not familiar enough with the differences between our appliances vs developer setup, so I'll leave that up to someone who knows...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose it is the change into 5.1, that uses yarn and webpack... I don't know about gulp, I just found it there.

Copy link

Choose a reason for hiding this comment

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

Copy link

@himdel himdel Jun 15, 2017

Choose a reason for hiding this comment

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

Right now, ops UI really needs only bower and npm.

SUI however needs webpack and yarn - those are only needed to build SUI, not to use it, but since this is a dev setup, I think they are needed here.

In the short future, ops UI will also need webpack and yarn and a will stop needing bower a while after that.

gulp-cli is probably not needed at all, since SUI stopped using gulp and ops UI never used it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can delete gulp-cli, and leave the rest there. Once we move into webpack I can delete bower too.

@chargio
Copy link
Contributor Author

chargio commented Jun 14, 2017

@Fryguy What is the correct version of ruby? I've seen the PR merged that allows 2.4.1 to be used.

ManageIQ/manageiq#13104

Should it be 2.3.3 or 2.3.1?

@Fryguy
Copy link
Member

Fryguy commented Jun 14, 2017

@sergio-ocon yeah, those changes are 2.2, 2.3, and 2.4 compatible changes to move us towards 2.4 support but we are not there yet. @jrafanie has a checklist going: ManageIQ/manageiq#14446 . That being said, many of us just use 2.4 locally anyway, and I haven't seen anything fail yet, but then again, I don't run the entire app locally either.

We use Ruby 2.3.1 in the appliances, so I would go with that for maximum compatibility. https://github.com/ManageIQ/manageiq-appliance-build/blob/master/kickstarts/partials/post/ruby_install.ks.erb#L10

@chargio
Copy link
Contributor Author

chargio commented Jun 20, 2017

@Fryguy I believe that this now is ok. I will merge it when you approve it.

@Fryguy Fryguy merged commit 4eec816 into ManageIQ:master Jun 20, 2017
@chargio chargio deleted the updating_requirements branch June 21, 2017 07:44
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.

5 participants