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

Demo fix #118

Closed
wants to merge 3 commits into from
Closed

Demo fix #118

wants to merge 3 commits into from

Conversation

srowhani
Copy link
Contributor

@srowhani srowhani commented Mar 3, 2017

This project uses semver, please check the scope of this pr:

  • #none# - documentation fixes and/or test additions
  • #patch# - backwards-compatible bug fix
  • #minor# - adding functionality in a backwards-compatible manner
  • #major# - incompatible API change

CHANGELOG

  • Change all_branches to only deploy on branch master
  • Edit publish-gh-pages script to deploy when called

From CI build 254
@coveralls
Copy link

coveralls commented Mar 3, 2017

Coverage Status

Changes Unknown when pulling 06e2ca4 on demo-fix into ** on master**.

@@ -51,7 +51,7 @@ deploy:
api_key:
secure: 2flghVwIh/AVn5hXljg4qjRdQfL7u/0PUiFvFumz8Q3UPkzsT3rQ+0GjSV6D9XLnnVNFK0KZ2eZEji1Zh2wht3V14FkHq95kRin1V82Xl69b0DD0S9fq9py/SLX3cR1Fr4FsCvCQL2VLkTmHoLZ+chiWHjR/Em0oro8/4tCfShk1mgQ8ZE7o3wnwItXRckPdtTj8Gzug3ce/lFmiHgKz9I8PFu9ErqkKmKQsitzuZwe58xRz3WOWsYrw3qNYjuC5TUnXKcHSwmI2F6WoAgO5latia2xryx9yosxCaZL7qie0kgW/lhk62FY1RwYzXuKfb4pW+XQBnJhpr7uBhq2K3HVWdYuUqTf/i6rYTIm2x9LuxkY25ok0V3vpKQDOwALEVSZpMMvNPRwyLWuUecNtNz73V47BNwETSo45dBCNgjhvu2Zh19WTHpHEgYacztlcUR/F9heRa1RDEKalS+XbtLX7NgyejJXARG9zb++G1d37wc3tESPge8Nwj5/ls3jN5M5pFE1/K7Ayus1MLBDsTBKLshmbfWKq7WbiXWiQtsou0PT0NJKojk+j7KCTYMg5nt79RF0OaRWsdog7FjE/KmnxDCjgQp8P1VEV0olvtDPbr52WUUVUofpNnV8V/70AI2eBkm4ZK0/8C2wBOKMH7XwYTVVyboaqwIpY+RDEi/Y=
on:
all_branches: true
branch: 'master'
Copy link

Choose a reason for hiding this comment

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

This is not a good idea for a number of reasons. First off, it prevents you from being able to publish a bugfix to a previous major, using a maintenance branch. Secondly, I believe it will prevent the package from ever publishing.

In order to support none in pr-bumper some changes were made to the way the builds work. Now, when a PR is merged, three builds kick off:

  1. The merge commit from the PR is built on the branch the PR is made into, this build does the pr-bumper bump step and the Automated version bump commit is made, which creates a tag with the new version
  2. The newly created commit is built from the target branch (usually master but maybe something like 0.x if there's a maintenance branch, this build is pretty much ignored, all the maybe-do-this.sh scripts look for the Automated version bump commit message and stop executing early if they see it.
  3. The actual new tag is built, this is one also skips a lot of things, like the tests, etc b/c of the maybe-do-something.sh scripts, but then satisfies the on clauses for deploy and finally publishes the package to npm.

With branch: 'master' and tags: true you're essentially making it so that the on is never satisfied.

echo "Skipping gh-pages publish for branch ${TRAVIS_BRANCH}"
exit 0
fi

Copy link

Choose a reason for hiding this comment

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

You still want this here, I think what you want to do is make the publish conditional as well, making sure it's not the bump commit, and moving it to happen before the version bump happens. I think @sandersky was gonna update ember-frost-bunsen to let it's demo work again with the latest pr-bumper configs, not sure if he got the chance to or not yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Bunsen is doing everything correctly except publishing to gh-pages. That'll be fixed once https://github.com/ciena-frost/ember-frost-bunsen/pull/371/files is merged.

* Fixed range select issues with selectedItems increasing with duplicates
* Simplified itemComparator to just itemKey: string


Copy link

Choose a reason for hiding this comment

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

I'm assuming this is b/c the previous build failed and so the bump commit wasn't made?

@coveralls
Copy link

coveralls commented Mar 3, 2017

Coverage Status

Changes Unknown when pulling 06e2ca4 on demo-fix into ** on master**.

@juwara0
Copy link
Contributor

juwara0 commented Mar 4, 2017

@srowhani - I have the demo app publishing to gh-pages now via: #120.

@juwara0
Copy link
Contributor

juwara0 commented Mar 4, 2017

I will try to make this change: https://github.com/ciena-frost/ember-frost-bunsen/pull/371/files#diff-354f30a63fb0907d4ad57269548329e3 that @sandersky mentioned next week.

@juwara0 juwara0 closed this Mar 4, 2017
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.

None yet

6 participants