search

umbraco / UmbPack

13 stars 13 forks source link

Current flag doesn't follow command line etiquette #32

Open mattbrailsford opened 4 years ago

mattbrailsford commented 4 years ago

It is currently supported that you can enable / disable the ability to set the package being uploaded as the current package which is controlled by the -c command line argument.

However, this argument is not following command line etiquette in that boolean options should be set or unset purely by their presence. Ie, currently, to control whether an upload should be made current or not you can do the following:

// Default is to make current
umbpack push -k [APIKEY] .\UmbPackTest_1.0.0.zip

// Explicitly make current
umbpack push -k [APIKEY] -c true .\UmbPackTest_1.0.0.zip

// Don't make the package current
umbpack push -k [APIKEY] -c false .\UmbPackTest_1.0.0.zip

Where as I think the proper use should be 

// Make current
umbpack push -k [APIKEY] -c .\UmbPackTest_1.0.0.zip

// Don't make  current
umbpack push -k [APIKEY] .\UmbPackTest_1.0.0.zip

I understand the reason for this is so that the default behavior is to make the package the current one, and to change it now would be a breaking change, but I think it would be better to be consistent with command line etiquette and have to be a bit more explicit about making current, than using a custom syntax in order to support an inverted boolean option.

jmayntzhusen commented 4 years ago

Hmm I see your point, maybe instead of a current flag we should have a "not current" flag - so you don't have to specify the flag just like now if you are pushing a current package, but you can add a flag saying this should not be the new current as you suggest 🤔

Struggling to think what it should be called though, this would be used both for alpha/beta versions and also for "addon" packages like your vendr providers and the forms files package we ship with Forms. Need a good name that fits both usecases.

mattbrailsford commented 4 years ago

Hmm, I think that's still trying to invert the boolean really. Honestly, I think the default behavior really should be to not set current, Really this is a "safer" option and you require explicit opt in to set current. If the default is to set current and you accidentally upload without knowing, all of a sudden your package is the default package people are downloading. I'd much rather this need to be explicit.