Continuum
  1. Continuum
  2. CONTINUUM-565

Forced builds should say who (which continuum user) forced the build

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.4.0 (Beta)
    • Component/s: Core system
    • Labels:
      None

      Description

      Currently if a build is forced the build history and notifications don't say who forced it. It would be nice if that was part of the history and notifications.

        Issue Links

          Activity

          Hide
          Wendy Smoak added a comment -

          Initial investigation

          DefaultBuildController.java has method
          public void build( int projectId, int buildDefinitionId, int trigger, ScmResult scmResult )

          'build' is called from BuildProjectTaskExecutor.java:

              public void executeTask( Task task ) throws TaskExecutionException {
                  BuildProjectTask buildProjectTask = (BuildProjectTask) task;
                  controller.build( buildProjectTask.getProjectId(), buildProjectTask.getBuildDefinitionId(), buildProjectTask.getTrigger(), buildProjectTask.getScmResult() );
              }
          
          • 'trigger' is an integer, no additional info such as username is passed in
          • Brett suggested 'trigger' should be a class rather than an int
          • username needs to get into BuildProjectTask so it's available in BuildProjectTaskExecutor when calling controller.build

          more in 20090514 #continuum irc log

          Show
          Wendy Smoak added a comment - Initial investigation DefaultBuildController.java has method public void build( int projectId, int buildDefinitionId, int trigger, ScmResult scmResult ) 'build' is called from BuildProjectTaskExecutor.java: public void executeTask( Task task ) throws TaskExecutionException { BuildProjectTask buildProjectTask = (BuildProjectTask) task; controller.build( buildProjectTask.getProjectId(), buildProjectTask.getBuildDefinitionId(), buildProjectTask.getTrigger(), buildProjectTask.getScmResult() ); } 'trigger' is an integer, no additional info such as username is passed in Brett suggested 'trigger' should be a class rather than an int username needs to get into BuildProjectTask so it's available in BuildProjectTaskExecutor when calling controller.build more in 20090514 #continuum irc log
          Hide
          Wendy Smoak added a comment -

          This should be solved by CONTINUUM-2265 (Need information on who executed a build or release)

          Show
          Wendy Smoak added a comment - This should be solved by CONTINUUM-2265 (Need information on who executed a build or release)
          Hide
          Wendy Smoak added a comment -

          2009-05-14 irc conversation from my logs:

          [4:56pm] wsmoak: does anyone know how anything actually gets built in Continuum?
          [4:56pm] wsmoak: all I want to do is add "by <whoever>" on the build results page to show who forced a build
          [4:57pm] brett: there's a build executor in core
          [4:57pm] brett: you probably want to do a find usages on the add method of the build result dao
          [4:57pm] wsmoak: BuildProjectTaskExecutor is one of the things I found
          [4:58pm] brett: that's him
          [4:58pm] wsmoak: all he does is call controller.build
          [4:59pm] wsmoak: BuildController is an interface
          [4:59pm] wsmoak: where's the code for pete's sake
          [4:59pm] brett:
          [4:59pm] wsmoak: there's no 'add' in the BuildResult class
          [4:59pm] brett: BR, or BRDao?
          [5:00pm] brett: if you're in eclipse, you can get an outline on the interface cass that shows subclasses
          [5:01pm] wsmoak: just BR. hadn't found the Dao one.
          [5:01pm] wsmoak: yeah, alt-F7
          [5:02pm] wsmoak: DefaultBuildController. possibly.
          [5:05pm] marica joined the chat room.
          [5:07pm] wsmoak: looks like it would mean changing a method signature on DefaultBuildController.build.
          [5:08pm] wsmoak: last time I tried something like that, it rippled through the entire app.
          [5:09pm] brett: yah
          [5:09pm] brett: it doesn't take a context of extra info?
          [5:11pm] wsmoak: not afaict - public void build( int projectId, int buildDefinitionId, int trigger, ScmResult scmResult )
          [5:12pm] brett: boo
          [5:12pm] brett: trigger should be a class
          [5:12pm] wsmoak: there's private void updateBuildResult( BuildContext context, String error ) that has a context...
          [5:12pm] brett: might only be called on error
          [5:14pm] wsmoak: darn. the build context gets created inside this class, it isn't passed in
          [5:14pm] wsmoak: so... turn Trigger into a class and see what else has to change...
          [5:14pm] brett: (everything)
          [5:14pm] brett:
          [5:15pm] wsmoak: yeah. here's the part where I decide changing stuff in continuum is way too hard and just go open more issues
          [5:15pm] brett: I would probably overload it, not change it
          [5:16pm] wsmoak: yeah. but I bet it's still going to be messy changing it on the other side where things are calling it.
          [5:18pm] wsmoak: hm. we may be in luck... the only usage I can find is the aforementioned controller.build(...) in BuildProjectTaskExecutor
          [5:19pm] wsmoak: which means BuildProjectTask has to change... and we'll leave figuring out who fills him up for another day
          [5:22pm] wsmoak: thanks

          Show
          Wendy Smoak added a comment - 2009-05-14 irc conversation from my logs: [4:56pm] wsmoak: does anyone know how anything actually gets built in Continuum? [4:56pm] wsmoak: all I want to do is add "by <whoever>" on the build results page to show who forced a build [4:57pm] brett: there's a build executor in core [4:57pm] brett: you probably want to do a find usages on the add method of the build result dao [4:57pm] wsmoak: BuildProjectTaskExecutor is one of the things I found [4:58pm] brett: that's him [4:58pm] wsmoak: all he does is call controller.build [4:59pm] wsmoak: BuildController is an interface [4:59pm] wsmoak: where's the code for pete's sake [4:59pm] brett: [4:59pm] wsmoak: there's no 'add' in the BuildResult class [4:59pm] brett: BR, or BRDao? [5:00pm] brett: if you're in eclipse, you can get an outline on the interface cass that shows subclasses [5:01pm] wsmoak: just BR. hadn't found the Dao one. [5:01pm] wsmoak: yeah, alt-F7 [5:02pm] wsmoak: DefaultBuildController. possibly. [5:05pm] marica joined the chat room. [5:07pm] wsmoak: looks like it would mean changing a method signature on DefaultBuildController.build. [5:08pm] wsmoak: last time I tried something like that, it rippled through the entire app. [5:09pm] brett: yah [5:09pm] brett: it doesn't take a context of extra info? [5:11pm] wsmoak: not afaict - public void build( int projectId, int buildDefinitionId, int trigger, ScmResult scmResult ) [5:12pm] brett: boo [5:12pm] brett: trigger should be a class [5:12pm] wsmoak: there's private void updateBuildResult( BuildContext context, String error ) that has a context... [5:12pm] brett: might only be called on error [5:14pm] wsmoak: darn. the build context gets created inside this class, it isn't passed in [5:14pm] wsmoak: so... turn Trigger into a class and see what else has to change... [5:14pm] brett: (everything) [5:14pm] brett: [5:15pm] wsmoak: yeah. here's the part where I decide changing stuff in continuum is way too hard and just go open more issues [5:15pm] brett: I would probably overload it, not change it [5:16pm] wsmoak: yeah. but I bet it's still going to be messy changing it on the other side where things are calling it. [5:18pm] wsmoak: hm. we may be in luck... the only usage I can find is the aforementioned controller.build(...) in BuildProjectTaskExecutor [5:19pm] wsmoak: which means BuildProjectTask has to change... and we'll leave figuring out who fills him up for another day [5:22pm] wsmoak: thanks
          Hide
          jzurbano added a comment - - edited

          Fixed in:
          r786863 in 1.3.x branch
          r786876 in trunk

          Show
          jzurbano added a comment - - edited Fixed in: r786863 in 1.3.x branch r786876 in trunk
          Hide
          Wendy Smoak added a comment -

          When we were discussing this on the list I wasn't aware this was going on the 1.3.x branch, and the issue was marked for "1.x" so no clue there either. As a new feature/improvement, IMO it should have only gone on trunk.

          Show
          Wendy Smoak added a comment - When we were discussing this on the list I wasn't aware this was going on the 1.3.x branch, and the issue was marked for "1.x" so no clue there either. As a new feature/improvement, IMO it should have only gone on trunk.
          Hide
          jzurbano added a comment -

          Wendy, I have overlooked the fix for version, my mistake. Should I revert the commit in 1.3.x branch instead?

          Show
          jzurbano added a comment - Wendy, I have overlooked the fix for version, my mistake. Should I revert the commit in 1.3.x branch instead?
          Hide
          jzurbano added a comment -

          Reverted r786863 commit (r786891).

          Show
          jzurbano added a comment - Reverted r786863 commit (r786891).
          Hide
          Wendy Smoak added a comment -

          Well I wouldn't have insisted on reverting it since it was already done.

          Thanks, though. I'm looking forward to seeing this in our first 1.4.x release!

          I see you updated the unit tests already; I'll see if I can find the right place in the Selenium tests if no one beats me to it.

          Show
          Wendy Smoak added a comment - Well I wouldn't have insisted on reverting it since it was already done. Thanks, though. I'm looking forward to seeing this in our first 1.4.x release! I see you updated the unit tests already; I'll see if I can find the right place in the Selenium tests if no one beats me to it.
          Hide
          Wendy Smoak added a comment -

          do any docs (esp. screen shots) need to be updated for this?

          Show
          Wendy Smoak added a comment - do any docs (esp. screen shots) need to be updated for this?
          Hide
          jzurbano added a comment -

          Wendy, yes. The build result screen shot should be replaced to reflect the added field (Triggered by).

          I will be updating the docs too.

          Thanks!

          Show
          jzurbano added a comment - Wendy, yes. The build result screen shot should be replaced to reflect the added field (Triggered by). I will be updating the docs too. Thanks!

            People

            • Assignee:
              jzurbano
              Reporter:
              Jamie Flournoy
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development