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

CB-13828: Improve cordova-coho/docs #170

Merged
merged 44 commits into from
Feb 23, 2018
Merged

CB-13828: Improve cordova-coho/docs #170

merged 44 commits into from
Feb 23, 2018

Conversation

janpio
Copy link
Member

@janpio janpio commented Jan 25, 2018

What does this PR do?

Improve readability of index.md/README.md:

  • Better headlines and grouping
  • Add missing files that were not linked before
  • Short introduction

Reworked platforms-release-process.md:

  • Table of Contents
  • More headlines with more structure, added "groups"
  • Tested all the commands, added notes and important information

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.
  • Added automated test coverage as appropriate for this change.

@janpio janpio changed the title [WIP] Improve cordova-coho/docs [WIP] CB-13828: Improve cordova-coho/docs Feb 20, 2018
@janpio janpio changed the title [WIP] CB-13828: Improve cordova-coho/docs CB-13828: Improve cordova-coho/docs Feb 21, 2018

### Repository setup

You should have your platform repository checked out in a folder where you als ohave checked out all/most/some of the other Cordova repositories. If you followed the [Cloning/Updating Cordova repositories
Copy link
Contributor

Choose a reason for hiding this comment

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

spelling: also have*


If the changes merit it, manually bump the major / minor/ patch version in `package.json`. View the changes via:
Note: In `cordova-android`, also remember to handle the version in `framework/build.gradle`.
Copy link
Contributor

Choose a reason for hiding this comment

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

169 can actually be removed. coho prepare-release-branch command that is run later will update build.gradle

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this good enough? I was somehow thinking there was a reason that "remove -dev" and the other version stuff later is split in 2 parts?

Copy link
Contributor

Choose a reason for hiding this comment

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

The remove dev part is split from the other version stuff because we use the version you set in package.json when writing to releasenotes. After you remove dev you may manually change the versionwhen reviewing commits in case you want a patch/minor/major release. This happens right before we run the update releasenotes command

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, now I finally got it I think. Updated the file to document this in more detail: efe62e3


for l in cordova-android; do ( cd $l; v="$(grep '"version"' package.json | cut -d'"' -f4)"; if [[ $v = *-dev ]]; then v2="${v%-dev}"; echo "$l: Setting version to $v2"; sed -i '' -E 's/version":.*/version": "'$v2'",/' package.json; fi) ; done

In `cordova-android`, also remember to bump the version in `framework/build.gradle`.
Note: This command [doesn't actually work](https://issues.apache.org/jira/browse/CB-13809). You can also replace `-dev` manually of course.
Copy link
Contributor

Choose a reason for hiding this comment

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

Strange, it worked fine for me on this latest android release.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I also felt this was strange as I assumes these were used quite often. You can take a look at the linked issues.


( cd cordova-android && git log --pretty=format:'* %s' --topo-order --no-merges $(git describe --tags $(git rev-list --tags --max-count=1))..master )

Note: This command [doesn't actually work](https://issues.apache.org/jira/browse/CB-13901). You can also check out the changes manually (or via the next step).
Copy link
Contributor

Choose a reason for hiding this comment

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

once again, worked fine for me for latest android release. A pre-requisite to running this command is making sure you have the latest tags. I had to run git fetch --tags in cordova-android repo to grab the tags I was missing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just noticed that I only ran this on Windows shell, not WSL/bash so this one might actually be just a different in the params of git on Windows.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, it works in WSL. Will update the text.


1. Creates a release branch `5.0.x` (if it doesn't already exist)
2. Updates `cordova.js` snapshot on both `5.0.x` and `master`
3. Updates version numbers (`VERSION` files) on `5.0.x`
Copy link
Contributor

Choose a reason for hiding this comment

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

It also updates in other files but that is platform specific. For example, it updates build.gradle version for android. It does similar stuff for iOS.

Copy link
Member Author

Choose a reason for hiding this comment

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

Reworded that and added your example.

Copy link
Contributor

@stevengill stevengill left a comment

Choose a reason for hiding this comment

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

LGTM

@janpio janpio merged commit 7d26af5 into master Feb 23, 2018
@janpio janpio deleted the janpio-CB-13828 branch February 23, 2018 23:49
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

2 participants