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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix generators issue #3684

Merged
merged 5 commits into from Jul 4, 2018

Conversation

@deivid-rodriguez
Copy link
Contributor

commented Jun 20, 2018

馃帺 What? Why?

This PR fixes a generator issue reported on gitter where bootsnap wouldn't be correctly added to the end application's Gemfile, and thus the generated application will fail to boot.

Tests are failing, but they're not failing for me locally when using decidim/docker#29 (that preinstalls decidim 0.12 in the base image). So I would like to merge that PR and use it here to see if the build is fixed.

I will forward-port to master later.

馃搶 Related Issues

馃搵 Subtasks

  • Add CHANGELOG entry

馃摲 Screenshots (optional)

None.

@ghost ghost added the status: WIP label Jun 20, 2018
@deivid-rodriguez deivid-rodriguez force-pushed the fix_generators_issue branch 3 times, most recently from 7e6911e to 0e69ed9 Jun 20, 2018
@mrcasals

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2018

Nice, thanks! I'll ping @josepjaume on that other PR, see if he can unblock and merge it 馃槃

@deivid-rodriguez

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2018

Thanks, @mrcasals! I already implemented his suggestion in that other PR, so I think it should be fine :)

@deivid-rodriguez deivid-rodriguez force-pushed the fix_generators_issue branch 3 times, most recently from ea6d12c to 7789bf3 Jun 24, 2018
@mrcasals mrcasals removed the status: WIP label Jun 25, 2018
@deivid-rodriguez

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2018

@mrcasals This is still WIP, the actual fix for the issue is not pushed, I was trying to get a failing test first. Hopefully once the docker image is updated this starts failing.

@deivid-rodriguez

This comment has been minimized.

Copy link
Contributor Author

commented Jun 25, 2018

Any news on unblocking that PR by the way?

@ghost ghost added the status: WIP label Jun 25, 2018
@josepjaume

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2018

PR unblocked! Apologies for the delay!

@mrcasals mrcasals removed the status: WIP label Jun 26, 2018
@mrcasals mrcasals dismissed their stale review Jun 26, 2018

Do not merge yet

@ghost ghost added the status: WIP label Jun 26, 2018
@mrcasals

This comment has been minimized.

Copy link
Contributor

commented Jun 26, 2018

@deivid-rodriguez it's failing, but I don't know if it's failing as you expected to... 馃槄

@deivid-rodriguez

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2018

Definitely not, generator specs still pass! So weird because they fail for me locally...

@deivid-rodriguez

This comment has been minimized.

Copy link
Contributor Author

commented Jun 26, 2018

Eureka! Got my failure! CircleCI caching was getting in the middle.

@mrcasals mrcasals referenced this pull request Jul 3, 2018
@deivid-rodriguez deivid-rodriguez force-pushed the fix_generators_issue branch from 90bc3f8 to a5c22e0 Jul 3, 2018
@deivid-rodriguez deivid-rodriguez changed the base branch from 0.12-stable to master Jul 3, 2018
@deivid-rodriguez deivid-rodriguez force-pushed the fix_generators_issue branch from a5c22e0 to 011cbd1 Jul 3, 2018
@deivid-rodriguez deivid-rodriguez force-pushed the fix_generators_issue branch from 011cbd1 to 2db1aed Jul 3, 2018
@deivid-rodriguez

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2018

I changed the target branch to master, rebased it, and converted in a follow up PR to #3756. It does the following:

  • Add a changelog entry for the missing bootsnap fix.
  • Fix another generator's issue, where optional gems were not being included.
  • Add regression tests.
  • Other minor related improvements.
@mrcasals mrcasals merged commit a41d0fd into master Jul 4, 2018
25 of 28 checks passed
25 of 28 checks passed
codeclimate/diff-coverage 60% (80% threshold)
Details
WIP work in progress
Details
codeclimate Code Climate is analyzing this code.
Details
ci/circleci: accountability Your tests passed on CircleCI!
Details
ci/circleci: admin Your tests passed on CircleCI!
Details
ci/circleci: api Your tests passed on CircleCI!
Details
ci/circleci: assemblies Your tests passed on CircleCI!
Details
ci/circleci: blogs Your tests passed on CircleCI!
Details
ci/circleci: budgets Your tests passed on CircleCI!
Details
ci/circleci: build_design_app Your tests passed on CircleCI!
Details
ci/circleci: build_test_app Your tests passed on CircleCI!
Details
ci/circleci: comments Your tests passed on CircleCI!
Details
ci/circleci: consultations Your tests passed on CircleCI!
Details
ci/circleci: core Your tests passed on CircleCI!
Details
ci/circleci: debates Your tests passed on CircleCI!
Details
ci/circleci: generators Your tests passed on CircleCI!
Details
ci/circleci: initiatives Your tests passed on CircleCI!
Details
ci/circleci: main Your tests passed on CircleCI!
Details
ci/circleci: meetings Your tests passed on CircleCI!
Details
ci/circleci: pages Your tests passed on CircleCI!
Details
ci/circleci: participatory_processes Your tests passed on CircleCI!
Details
ci/circleci: proposals Your tests passed on CircleCI!
Details
ci/circleci: sortitions Your tests passed on CircleCI!
Details
ci/circleci: surveys Your tests passed on CircleCI!
Details
ci/circleci: system Your tests passed on CircleCI!
Details
ci/circleci: upload-coverage Your tests passed on CircleCI!
Details
ci/circleci: verifications Your tests passed on CircleCI!
Details
codeclimate/total-coverage 98% (0.0% change)
Details
@ghost ghost removed the status: WIP label Jul 4, 2018
@mrcasals mrcasals deleted the fix_generators_issue branch Jul 4, 2018
mrcasals added a commit that referenced this pull request Jul 5, 2018
* Add change log entry

* Move `with_original_env` out of generators

* Remove unexistent file from rubocop exclusions

* Let binstubs respect current folder

* Fix generated apps not including optional plugins
mrcasals added a commit that referenced this pull request Jul 10, 2018
* Add change log entry

* Move `with_original_env` out of generators

* Remove unexistent file from rubocop exclusions

* Let binstubs respect current folder

* Fix generated apps not including optional plugins
mrcasals added a commit that referenced this pull request Jul 10, 2018
* Add change log entry

* Move `with_original_env` out of generators

* Remove unexistent file from rubocop exclusions

* Let binstubs respect current folder

* Fix generated apps not including optional plugins
josepjaume added a commit that referenced this pull request Jul 10, 2018
* More consistent circle ci env (#3732)

* Remove unnecessary env on Circle CI

This was a workaround for a bundler bug that should no longer be
happening.

* Run tests through bundler

* Pass SIMPLECOV env to docker specs

To more exactly simulate CircleCI environment.

* Remove npm caching

* Block sassc dependency version (#3775)

Fix dependencies

* Update deps

* Revert "Flexibilize internal dependencies"

This reverts commit 434e93c.

* Update deps

* Fix deps

* Run all generators tests to see what fails

* Test generators against the very same branch

* Update more deps

* Fix generators issue (#3684)

* Add change log entry

* Move `with_original_env` out of generators

* Remove unexistent file from rubocop exclusions

* Let binstubs respect current folder

* Fix generated apps not including optional plugins

* Restore RUBYLIB after loading the right version

* Revert "Run all generators tests to see what fails"

This reverts commit 4e6de3d.

* Revert "Test generators against the very same branch"

This reverts commit 4430c3c.

* Update deps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.