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-11279 Support arbitrary repositories for 'coho nightly' #122

Closed
wants to merge 5 commits into from
Closed

CB-11279 Support arbitrary repositories for 'coho nightly' #122

wants to merge 5 commits into from

Conversation

vladimir-kotikov
Copy link
Member

This PR adds the following features to coho nightly command:

  • it now accepts --repo flag to specify repos to build nightlies for (instead of using predefined repos) . If --repo is not specified, cordova-cli and cordova-lib will be built.
  • it now updates dependencies for every package to point to newly released nightly versions
  • it handles the situation when the repo is not yet published under nightly tag gracefully

}
yield repoutil.forEachRepo(reposToBuild, function*(repo) {
var packageJSONPath = path.join(process.cwd(), 'package.json');
var packageJSON = require(packageJSONPath);
Copy link
Contributor

@nikhilkh nikhilkh May 17, 2016

Choose a reason for hiding this comment

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

For platform repos we maintain other files that have the version as well - like for cordova-android there is the VERSION file. How and when will they get updated? Perhaps at some point we should fix that to only use package.json for versions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, there is the version script too - I believe coho has methods to update the version. For now we might have to just use them. But we should file a JIRA to clean that up to reduce to one source of version from package.json

Copy link
Member Author

Choose a reason for hiding this comment

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

@nikhilkh, updated, please take a look once more. Respective JIRA issue is CB-11290

Copy link
Contributor

Choose a reason for hiding this comment

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

So do the other places where the version is stored - not need to be updated with the nightly version?

Copy link
Member Author

Choose a reason for hiding this comment

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

They now get updated by versionutil.updateRepoVersion - it handles all possible places, where version needs to be written to (see https://github.com/apache/cordova-coho/pull/122/files#diff-91127a766407aac9c0bc1bf9da171917R75) This method was a part of platform-release module before but i decided to move it to versionutil as it is IMO more appropriate place.

} else {
packageJSON.dependencies['cordova-lib'] = cordovaLibVersion;
}
var packageJSONPath = path.join(process.cwd(), 'package.json');
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like updateRepoVersion already updates package.json - you can probably remove the code here.

Copy link
Member Author

Choose a reason for hiding this comment

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

We still need to update dependencies in package.json in case if we're releasing both dependent package and its dependency (e.g. cordova-lib and cordova-common), so the code is still required.

Anyway this code will be updated in the next PR that adds more robust support for platforms nightly releases

@nikhilkh
Copy link
Contributor

One minor comment - LGTM otherwise

@asfgit asfgit closed this in 94bf9d8 May 19, 2016
@vladimir-kotikov vladimir-kotikov deleted the CB-11279 branch May 19, 2016 08:28
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