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

[WIP] CB-14249 ensure platform changes are in a dev version (partial workaround doc fix) #189

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

brodycj
Copy link

@brodycj brodycj commented Aug 2, 2018

Platforms affected

All

What does this PR do?

Add section with note to ensure that changes are made in a "-dev" version.

This is a partial workaround for the fundamental issue in CB-14249, contributed in response to the discussion in apache/cordova-android#469. This kind of workaround was already applied in apache/cordova-android#470, apache/cordova-ios#379, apache/cordova-android#454, and several other places.

I hope we can resolve the inconsistency in release procedure for platforms vs tools described in CB-14249 someday, with agreement via dev@cordova.apache.org. I cannot promise when I will get a chance to take care of this.

TODO:

  • get rid of the "manually" option
  • show the needed coho command

Unfortunately I think I need some of the changes from #188 to make the coho prepare-platform-release-branch command work as needed on the non-master branch. I hope I get a chance to cleanup some of the changes from #188 and fix this one in the near future.

Merge procedure

  • Should be merged by squash (second commit is there to splve a problem with the first commit)

What testing has been done on this change?

  • Quick visual check

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected. (not true for second commit, which should be squashed anyway)
  • Added automated test coverage as appropriate for this change.

@brodycj brodycj self-assigned this Aug 2, 2018
@brodycj brodycj requested a review from janpio August 2, 2018 18:38
@@ -122,6 +123,10 @@ Double check you replace "Android" in the subject and mail body - there is no un

Note that it would be possible to continue with some of the [Before Release](#before-release) items while waiting for a possible response.

### Ensure changes are in a dev version

The procedure described here may leave non-master branch with a non-dev version number. Functional changes should always be done in a "-dev" version. Please mark the "-dev" version manually or using cordova-coho before making changes, and test it especially when marking "-dev" version manually.
Copy link
Member

Choose a reason for hiding this comment

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

What is the cordova-coho command to use?
Where does one have to change things when doing this manually?

Copy link
Member

Choose a reason for hiding this comment

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

(Sorry about being pedantic here, but I guess you probably have this information handy)

@brodycj brodycj changed the title CB-14249 ensure platform changes are in a dev version (partial workaround doc fix) [WIP] CB-14249 ensure platform changes are in a dev version (partial workaround doc fix) Aug 2, 2018
@brodycj
Copy link
Author

brodycj commented Aug 2, 2018

Thanks @janpio for the feedback. I would definitely agree that the command is missing and that we should get rid of "manually". The thing is that I think we really need some of the changes I proposed in #188 for the coho command to work in the release branch. I just updated the description with this info.

There a few things I want to do first, major one is to finish the Cordova patch release before cleaning up #188. Keeping this one as "WIP" for now.

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.

2 participants