Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 330197

Summary: [Build] Tidy up Build Configurations API
Product: [Eclipse Project] Platform Reporter: James Blackburn <jamesblackburn+eclipse>
Component: ResourcesAssignee: James Blackburn <jamesblackburn+eclipse>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: arthorne.eclipse, Szymon.Brandys
Version: 3.7Flags: Szymon.Brandys: review+
Target Milestone: 3.7 M4   
Hardware: PC   
OS: All   
Whiteboard:
Bug Depends on: 325489    
Bug Blocks: 331031    
Attachments:
Description Flags
patch 1
Szymon.Brandys: iplog+
patch 2 Szymon.Brandys: iplog+

Description James Blackburn CLA 2010-11-14 06:55:23 EST 
There are a couple API tweaks we might like to make before people start relying on the new BuildConfigurations support.

Rename: IBuildConfiguration.getId() -> getName()

We removed the name attribute from the build configuration, leaving the unique Id.  The reason for an Id, rather than name, was to prevent references to build configurations breaking when users changed the human-readable name of the configuration.  
However as we don't persist the build configurations in the .project, there's no real risk of build configurations going stale in a repository. We could just leave it up to the integrator to fix-up the references when the name changes.  UI code can then display configuration names (when there are multiple configurations) without issue, and getName is more inline with the rest of core.resources.

Change: IProjectDescription.setBuildConfigurations(IBuildConfiguration[]) -> setBuildConfigurations(String[])

Szymon points out that without any additional metadata we can directly set String[] of configuration names.

Anything else?
Comment 1 Szymon Brandys CLA 2010-11-15 05:24:41 EST 
(In reply to comment #0)
> Anything else?

We could consider these changes:

- IProject#getReferencedBuildConfigurations(IBuildConfiguration config, boolean includeMissing) to IProject#getReferencedBuildConfigurations(String configName, boolean includeMissing)

- Remove IWorkspace#newBuildConfiguration

- And minor issue. Sometimes we use BuildConfiguration, like in IProject#getReferencedBuildConfigurations and sometimes it is BuildConfig like in IprojectDescription#setBuildConfigReferences. I would use BuildConfig(s) everywhere. 

One more thing is that we use 'configuration' for two different capabilities in builders. Historically the first is about customizing of what build triggers a builder may respond too. The second one is the newly introduced one. I wonder if we can make something about it?
Comment 2 James Blackburn CLA 2010-11-28 09:56:56 EST 
Created attachment 183997 [details]
patch 1

Small(-ish) API tidy patch.

IBuildConfiguration: getId -> getName
IProjectDescription:
  *BuildConfiguration() -> *BuildConfig()
  setBuildConfigs(IBuildConfiguration[]) -> setBuildConfigs(String[])
IProject:
  hasBuildConfig(String)  
  getReferencedBuildConfigs(String)

(In reply to comment #1)
> - Remove IWorkspace#newBuildConfiguration

Not sure this can be easily done.  It's useful to easily be able to create build configurations for setting up the references.

> One more thing is that we use 'configuration' for two different capabilities in
> builders. Historically the first is about customizing of what build triggers a
> builder may respond too. The second one is the newly introduced one. I wonder
> if we can make something about it?

To me the two options on the builder extension point seem distinct: isConfigurable dictates whether the builder is configurable for triggers, and supportsConfigurations says whether the builder supports build configurations.   
I'm all ears to any ideas on better naming.
Comment 3 Szymon Brandys CLA 2010-11-29 04:32:48 EST 
I fixed some non-javadoc comments and argument names in some internal methods too. The patch with these changes is in HEAD.
Comment 4 Szymon Brandys CLA 2010-11-29 04:35:00 EST 
James, I think you used the name 'build variants' for this new concept. This may be an alternative for 'build configurations'. But on the second thought we can go either way. It's up to you.
Comment 5 Szymon Brandys CLA 2010-11-29 06:59:24 EST 
There are still some internal methods that need renaming. When it is done, we will close the bug.
Comment 6 James Blackburn CLA 2010-11-29 07:00:26 EST 
Created attachment 184026 [details]
patch 2

More #*Configuration(s) => #*Config(s)
References to config Id => config name
Comment 7 Szymon Brandys CLA 2010-11-29 07:11:11 EST 
Looks good. Thanks James!