Merge lp://staging/~rexbron/bzr-builddeb/trunk.profiles into lp://staging/~bzr-builddeb-hackers/bzr-builddeb/trunk-old

Proposed by Andrew Hunter
Status: Work in progress
Proposed branch: lp://staging/~rexbron/bzr-builddeb/trunk.profiles
Merge into: lp://staging/~bzr-builddeb-hackers/bzr-builddeb/trunk-old
To merge this branch: bzr merge lp://staging/~rexbron/bzr-builddeb/trunk.profiles
Reviewer Review Type Date Requested Status
James Westby Needs Fixing
Review via email: mp+2535@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Andrew Hunter (rexbron) wrote :

Adds support for profiles in bzr-builddeb. Profiles are stored groups of settings, callable from the command line. This is useful when multiple builder strings are required depending on the destination.

305. By Andrew Hunter

Fixed bug caused by if/else indentation errors.

306. By Andrew Hunter

Removed useless info().

Revision history for this message
James Westby (james-w) wrote :

Hi,

Thanks for working on this, apologies for the delay in reviewing it.

The change looks small and un-obtrusive, I just have a question about
the config file format. You went for

  [revu]
  profile-builder = "foo"

I think this should be

  [revu]
  builder = "foo"

However I was expecting

  [PROFILES]
  revu = "foo"
  ppa = "bar"

etc.

Your way obviously allows us to define other things for a profile, but I am
struggling to think of what that might be. Do you have any examples?

Also, what do you think about case-sensitivity. I think the profile names should
be case-insensitive, but that is obviously not that easy to accomplish. How
important do you think that is?

Thanks,

James

review: Needs Fixing
Revision history for this message
Andrew Hunter (rexbron) wrote :

Two reasons that I went with the config file format that I did, the first was namespace conflicts. I wanted to be certain, as I do not fully understand how this config system works, that I would not be interfering or causing unintentional changes. Secondly, as you mention, it might be possible in the future that we wish to add other options to a profile but I do not have any explicit examples at this point.

Making profile names case insensitive depends on how the config system works when it parses the config file. I have done some work on managing config files from python (see ubuntustudio-controls). If it uses regex to find the section header, then it would be a simple matter of modifying the regex passed to be case-insensitive. If it parse config files in a different manner, then I would need to think further, but I do agree that section headers should be case-insensitive.

Revision history for this message
James Westby (james-w) wrote :

Hi,

Sorry for the delay, I merged your branch with trunk and made some
tweaks:

  https://code.edge.launchpad.net/~james-w/bzr-builddeb/profiles

though I realise I forgot to change README.

I'm quite happy that the change is small. My remaining concern is
just utility vs. maintainence burden. I want to be sure that this
solves a use case well enough that it's worth the effort of maintaining
it. If you can explain what you want to use it for that will help.

Thanks,

James

Unmerged revisions

306. By Andrew Hunter

Removed useless info().

305. By Andrew Hunter

Fixed bug caused by if/else indentation errors.

304. By Andrew Hunter

Fixed typo and added profile shortname to help message.

303. By Andrew Hunter

Added test to make sure profile name gets pulled correctly.

302. By Andrew Hunter

Catch bad profile name exception.

301. By Andrew Hunter

Fixed typo

300. By Andrew Hunter

Added builder-profile section to README and added short name for profile.

299. By Andrew Hunter

Added a method to specify builder profiles in conf files.

Subscribers

People subscribed via source and target branches