Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 UTC on 18 March 2024, to get $100 off your ticket.
It looks like drush could support using a flag of --version-number or --increment-version when running drush features-update. Would such a patch get accepted? Any trouble spots to watch out for?
Comment | File | Size | Author |
---|---|---|---|
#38 | features-1272586-38.patch | 1.04 KB | TravisCarden |
#34 | features-1272586-34.patch | 1.12 KB | TravisCarden |
#32 | features-1272586-32.patch | 1.13 KB | alexweber |
#27 | features-1272586-final.patch | 5.37 KB | alexweber |
#23 | features-1272586-final.patch | 6.71 KB | alexweber |
Comments
Comment #1
febbraro CreditAttribution: febbraro commentedI think --version-number would be the way to go. If you want to look at implementing a patch I'll review it.
Comment #2
ahwebd CreditAttribution: ahwebd commentedThis would be great,
I'm not using "drush features-update" at the moment only because it can't set a version
Comment #3
Grayside CreditAttribution: Grayside commented@ahwebd You realize version number is trivial to set in the .info file?
I agree, --version-number seems good. But really, where does it stop? Maybe there should be a master flag to just set things into the .info file. That way drush fu could set the package, or kit compliance, random additional dependencies, etc.
--info-version=6.x-2.3 --info-dependency=spaces --info-dependency=admin
Comment #4
longwavedrush features-add feature_module dependencies:spaces
can be used to add dependencies in the -dev version, a separate switch isn't needed.Comment #5
longwaveOf course any version number handling in features-update should also be applied when using features-add :)
Comment #6
hefox CreditAttribution: hefox commentedMarked #1406166: Allow Drush to update features' version as duplicate
Comment #7
alexweber CreditAttribution: alexweber commentedI'm very interested in this issue as I hate manually updating my info files every time :)
Seems simple enough, I'll try to work on a patch later on today.
Comment #8
alexweber CreditAttribution: alexweber commentedSince drush_features_update() calls _drush_features_export(), which is also called by other functions, this functionality would end up being semi-global and the --version option would be available to:
More on this soon! :)
Comment #9
alexweber CreditAttribution: alexweber commentedOk, this was actually easier than it looked! :)
Attached is a patch that implements this. The option is --version. (--version-number is kinda longwinded to type)
Caveat: there is no validation in place for the version number (yet).
Comment #10
alexweber CreditAttribution: alexweber commentedOk, this patch now uses the same validation regex as features.admin.inc and prints a notice and dies on invalid version numbers.
Afaik its good to go, can anyone else test this please? (patch against -dev release)
Thanks! :)
Comment #11
alexweber CreditAttribution: alexweber commentedAttached is an alternative patch that extends the functionality of the previous patch to allow automatically incrementing a feature's version number.
Setting a feature number manually:
drush fu featurename --version=7.x-1.0
Setting a feature number automatically:
drush fu featurename --version=increment
Feedback anyone?
Comment #12
alexweber CreditAttribution: alexweber commentedoops :)
Comment #13
davidcsonka CreditAttribution: davidcsonka commentedYes, please. :)
I'm not really using drush to manage feature updates until this functionality is in. The version numbers is important to me and my development process
Comment #14
alexweber CreditAttribution: alexweber commentedI realize the feature devs are probably really busy but it'd be great to have someone review and hopefully commit this patch. I've tested it a lot and it's pretty solid IMO. :)
Thanks!
Comment #15
joelcollinsdc CreditAttribution: joelcollinsdc commentedI sort of agree with #3. There are a whole host of things that can be specified in the .info file. Description, package, kit/debut specs, etc. Adding options to drush fu doesn't seem like the most sustainable solution.
What about a drush command to open the .info file for the feature in a text editor?
drush features-edit featurename?
Comment #16
Grayside CreditAttribution: Grayside commentedI'd say what's really needed is to take the work going on in issues like #1331278: Drush features-add and features-export are confusingly similar, and overly verbose a step further, and allow arbitrary component additions via command-line parameter. Afterall, adding a component to a feature for export is really just a modification to the $info array constructed or loaded into memory before it's processed by the export routines. That info array is then written out to the .info file.
Comment #17
alexweber CreditAttribution: alexweber commentedfebrraro said "I think --version-number would be the way to go. If you want to look at implementing a patch I'll review it.".
It's implemented in #10.
Grayside I agree that there's a million different and potentially better ways about going with any implementation but this is a much needed and simple feature. I dont't understand why we can't just accept this solution for now and move on to greater things in the long run?
Anyone who works with features knows what a pain it is to have to manually increment version numbers after updating with drush fu.
My 2 cents.
Comment #18
joelcollinsdc CreditAttribution: joelcollinsdc commentedThis does seem to be a useful patch, thanks alex for your work. I realize the below is mostly bikeshedding...
1) The --version option does not seem to conflict with the global drush --version option, but it seems like a best practice would be to not choose options that match global drush options.
2) When you choose --increment, if your existing version is something like 7.x-1.0-dev, it becomes 7.x-1.0-dew. I realize this is a corner case. It works if your version is something like 7.x-1.0-beta1 (becomes beta2). Not sure if you intended this though.
Comment #19
alexweber CreditAttribution: alexweber commentedHi Joel, good call, I should have checked for is_numeric() before incrementing.
Any suggestions on what the option name should be?
Comment #20
alexweber CreditAttribution: alexweber commentedAttached is a patch that fixes the minor issue spotted by joelcollinsdc in #18
I'd be happy to refactor the option name if a better suggestion exists, I can't think of anything at the moment that would be different than "--version" without being confusing. Perhaps using "--increment", but this would break the request that febbraro made, of allowing the version to be manually set.
Comment #21
joetsuihk CreditAttribution: joetsuihk commentedmy little thoughts on option name, --version=increment is confusing, i suggest increment use another option: --version-increment
set version directly: --version-set=7.x-1.0-dev
Comment #22
alexweber CreditAttribution: alexweber commented@joetsuihk I like this, this way we stay clear of the global drush --version option but still maintain legible and meaningful names! :)
Will re-roll the patch in a few minutes.
Comment #23
alexweber CreditAttribution: alexweber commentedOk, this is a (hopefully) final patch for this issue. It was rewritten from scratch against the 7.x-1.x branch and should apply much cleaner than the previous patch which was getting unwieldly with so many commits.
It does the following:
Validation rules are in place to prevent invalid version strings when using --version-set and also to only increment versions using --version-increment when the version ends in a number. IE: dev releases don't get incremented but alpha, beta & rc do get incremented.
The reason for adding the funtionality to features-add and features-export is a natural consequence because the logic is implemented in _drush_features_export().
Comment #24
alexweber CreditAttribution: alexweber commentedCan any of the maintainers take a look at this please, its been RTBC for a while now! Thanks! :)
Comment #25
mpotter CreditAttribution: mpotter commented@alexweber: please reroll this into a proper patch format (remove your email headers, etc) on the latest 7.x-1.x-dev version. You can create a patch using the command:
git diff > features-1272586-final.patch
Not sure what other method you are using, but you'll get your patches reviewed and submitted more quickly if you follow the correct formats.
Comment #26
alexweber CreditAttribution: alexweber commented@mpotter, I was using git format-patch. Will do thanks.
Comment #27
alexweber CreditAttribution: alexweber commentedAttached is a much cleaner patch. Description is exactly the same as specified in #23.
Comment #28
mpotter CreditAttribution: mpotter commentedGreat! Sorry it took so long to get back to this. Committed and pushed to 0e735f1.
Comment #29
alexweber CreditAttribution: alexweber commented@mpotter, awesome! :)
Comment #30
myselfhimself CreditAttribution: myselfhimself commentedHello,
with version 7.x-1.0, when using drush feature-export with --version-increment, if former version number is 7.x-1.0-beta9, it gets incremented to 7.x-1.0-betb0.
Sorry not to propose a patch for this myself now.
Cheers
Comment #31
hefox CreditAttribution: hefox commentedPlease file a new bug report ^^
Comment #32
alexweber CreditAttribution: alexweber commentedThanks for finding this!
Patch attached.
Disclaimer: It's not that pretty but gets the job done without hardcoding minor version prefixes (alpha, beta, rc, etc). Improvements are welcome!
Comment #33
alexweber CreditAttribution: alexweber commentedcan anyone take a look at this? i've been running with this patch in my makefile with no issues for the past couple months but i cant RTBC it cause its my work :)
Comment #34
TravisCarden CreditAttribution: TravisCarden commentedThat works great, @alexweber. It fixes the problem on the 1.x and the 2.x branches. The patch applies to the 2.x branch, too, if you use
git apply --ignore-whitespace
, but for the sake of the committer, here's a reroll (attached). It involves nothing but a change in the indentation level. So to be clear, this patch applies to 7.x-2.x, and #32 can be committed to 7.x-1.x. Hopefully I'm not bending the rules too much. ;-)Comment #35
TravisCarden CreditAttribution: TravisCarden commentedBtw, follow-up: #1979170: Add --version-increment and --version-set support to drush features-update-all.
Comment #36
alexweber CreditAttribution: alexweber commented@Travis, that's fine thanks for cleaning it up! As long as it works and everyone's happy, so am I :)
Comment #37
myselfhimself CreditAttribution: myselfhimself commentedHey @alexweber, @TravisCarden thank you very much for your last patch !!!!
Comment #38
TravisCarden CreditAttribution: TravisCarden commentedOh dear; I let a
drush_print_r()
sneak into that last patch! Fix attached. As before, it applies to 7.x-2.x, and #32 can be committed to 7.x-1.x.Comment #39
alexweber CreditAttribution: alexweber commentedgo testbot!
Comment #40
mpotter CreditAttribution: mpotter commentedCommitted to 2b4ac6a.