You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by dblotsky <gi...@git.apache.org> on 2015/04/07 02:04:54 UTC
[GitHub] cordova-medic pull request: [CB-8805] Added ability to specify a c...
GitHub user dblotsky opened a pull request:
https://github.com/apache/cordova-medic/pull/41
[CB-8805] Added ability to specify a custom repos config file via forced builds.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-medic cb-8805
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-medic/pull/41.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #41
----
commit c7a8977f8d280d1b936697f9ce9f23eb691e426f
Author: Dmitry Blotsky <dm...@gmail.com>
Date: 2015-03-31T11:43:24Z
[CB-8805] Added ability to specify a custom repos config file via forced builds.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic pull request: [CB-8805] Added ability to specify a c...
Posted by dmitriy-barkalov <gi...@git.apache.org>.
Github user dmitriy-barkalov commented on the pull request:
https://github.com/apache/cordova-medic/pull/41#issuecomment-91146210
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic pull request: [CB-8805] Added ability to specify a c...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-medic/pull/41
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic pull request: [CB-8805] Added ability to specify a c...
Posted by dmitriy-barkalov <gi...@git.apache.org>.
Github user dmitriy-barkalov commented on a diff in the pull request:
https://github.com/apache/cordova-medic/pull/41#discussion_r28042404
--- Diff: buildbot-conf/cordova.conf ---
@@ -234,8 +245,15 @@ plugins_cleanup_steps = [
common_plugins_steps = plugins_cleanup_steps + get_medic_steps + [
# download medic's config to slave
- Download(mastersrc=PROJECTS_CONFIG_FILE, slavedest='cordova-medic/cordova-repos.json'),
- Download(mastersrc=MEDIC_CONFIG_FILE, slavedest='cordova-medic/config.json'),
+ Download(mastersrc=MEDIC_CONFIG_FILE, slavedest='cordova-medic/config.json', description='downloading master\'s config'),
+
+ # download repo config
+ # NOTE:
+ # only one of these steps should be executed; thanks
+ # to Buildbot there is no good if-else construct for
+ # builds, so we have two steps with 'doStepIf's
+ SH(command=['curl', P(REPOS_PROPERTY_NAME), '--output', 'cordova-medic/cordova-repos.json'], description='downloading custom repo config', doStepIf=dont_use_default_repos),
+ Download(mastersrc=PROJECTS_CONFIG_FILE, slavedest='cordova-medic/cordova-repos.json', description='downloading default repo config', doStepIf=use_default_repos),
--- End diff --
Thanks or clarification. Lets keep this approach.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic pull request: [CB-8805] Added ability to specify a c...
Posted by dmitriy-barkalov <gi...@git.apache.org>.
Github user dmitriy-barkalov commented on a diff in the pull request:
https://github.com/apache/cordova-medic/pull/41#discussion_r27996739
--- Diff: buildbot-conf/cordova.conf ---
@@ -234,8 +245,15 @@ plugins_cleanup_steps = [
common_plugins_steps = plugins_cleanup_steps + get_medic_steps + [
# download medic's config to slave
- Download(mastersrc=PROJECTS_CONFIG_FILE, slavedest='cordova-medic/cordova-repos.json'),
- Download(mastersrc=MEDIC_CONFIG_FILE, slavedest='cordova-medic/config.json'),
+ Download(mastersrc=MEDIC_CONFIG_FILE, slavedest='cordova-medic/config.json', description='downloading master\'s config'),
+
+ # download repo config
+ # NOTE:
+ # only one of these steps should be executed; thanks
+ # to Buildbot there is no good if-else construct for
+ # builds, so we have two steps with 'doStepIf's
+ SH(command=['curl', P(REPOS_PROPERTY_NAME), '--output', 'cordova-medic/cordova-repos.json'], description='downloading custom repo config', doStepIf=dont_use_default_repos),
+ Download(mastersrc=PROJECTS_CONFIG_FILE, slavedest='cordova-medic/cordova-repos.json', description='downloading default repo config', doStepIf=use_default_repos),
--- End diff --
I think it is over engineering to use conditional steps here. You have to implement 2 extra functions for this. Also it is confusing to user to always see two "downloading repo config" steps every time (one of them always skipped).
In this particular place function like this could be used:
```
def IfElse(condition, thenSteps, elseSteps):
if (condition):
return thenSteps
else:
return elseSteps
```
...
```
IfElse(P(REPOS_PROPERTY_NAME) is None, Download(mastersrc=PROJECTS_CONFIG_FILE, slavedest='cordova-medic/cordova-repos.json', description='downloading default repo config'), Download(mastersrc=PROJECTS_CONFIG_FILE, slavedest='cordova-medic/cordova-repos.json', description='downloading default repo config')),
```
This way there will be one step in build downloading repos from right place.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic pull request: [CB-8805] Added ability to specify a c...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on the pull request:
https://github.com/apache/cordova-medic/pull/41#issuecomment-94092731
Ping.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-medic pull request: [CB-8805] Added ability to specify a c...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-medic/pull/41#discussion_r28000339
--- Diff: buildbot-conf/cordova.conf ---
@@ -234,8 +245,15 @@ plugins_cleanup_steps = [
common_plugins_steps = plugins_cleanup_steps + get_medic_steps + [
# download medic's config to slave
- Download(mastersrc=PROJECTS_CONFIG_FILE, slavedest='cordova-medic/cordova-repos.json'),
- Download(mastersrc=MEDIC_CONFIG_FILE, slavedest='cordova-medic/config.json'),
+ Download(mastersrc=MEDIC_CONFIG_FILE, slavedest='cordova-medic/config.json', description='downloading master\'s config'),
+
+ # download repo config
+ # NOTE:
+ # only one of these steps should be executed; thanks
+ # to Buildbot there is no good if-else construct for
+ # builds, so we have two steps with 'doStepIf's
+ SH(command=['curl', P(REPOS_PROPERTY_NAME), '--output', 'cordova-medic/cordova-repos.json'], description='downloading custom repo config', doStepIf=dont_use_default_repos),
+ Download(mastersrc=PROJECTS_CONFIG_FILE, slavedest='cordova-medic/cordova-repos.json', description='downloading default repo config', doStepIf=use_default_repos),
--- End diff --
I wish it worked that way, but sadly [it does not](http://docs.buildbot.net/current/manual/cfg-properties.html#using-properties-in-steps). :disappointed: The way to do what you're talking about is to implement a [custom step that runs shell commands](http://docs.buildbot.net/current/manual/customization.html#running-commands), which is just as convoluted.
The way to solve it properly is to have only one download step and use a *default* property value, like so:
`P('repositories_config', default=PROJECTS_CONFIG_FILE)`
The issue right now is that the default file uses `FileDownload`, while the custom file uses `ShellCommand(command=['curl', ...])`, and making both use the same one is also a convoluted process. This will be fixed entirely once we switch to using [Buildbot Codebases](http://docs.buildbot.net/current/manual/concepts.html#multiple-codebase-builds) for managing multiple repositories.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org