[ 
https://issues.apache.org/jira/browse/CB-8805?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14485678#comment-14485678
 ] 

ASF GitHub Bot commented on CB-8805:
------------------------------------

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.


> Allow specifying repo versions for on-demand medic builds
> ---------------------------------------------------------
>
>                 Key: CB-8805
>                 URL: https://issues.apache.org/jira/browse/CB-8805
>             Project: Apache Cordova
>          Issue Type: Improvement
>          Components: Medic
>    Affects Versions: Master
>            Reporter: Dmitry Blotsky
>            Priority: Critical
>   Original Estimate: 168h
>  Remaining Estimate: 168h
>
> Right now medic only does builds from master for multi-repo builds (e.g. 
> plugins builds). There should be a way to specify the version of each repo 
> that goes into a build. This is needed for release testing and for PR testing.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to