change .dirty to -dirty

[derenrich]

This makes versions created by this plugin fully compatible with the SLS spec (I'd link but that repo is no longer public). This commit is a result of a discussion on the SLS spec.

Probably should bump the major version of this plugin after this commit since it could break people's workflows.

[ilyanep]

+2 lgtm

[uschi2000]

sgtm

[derenrich]

who presses the merge button and cuts the release if not you?

[markelliot]

Sorry, I'm confused, why do we want dirty versions to be valid versions in our deployability ecosystem?

[derenrich]

This ticket is not asking for support for dirty versions. We already have support explicitly called out. See for example test cases in sls-packaging. The spec for the manifest specifically calls out dirty version strings as valid.

I have reasons for wanting this. They don't involve publishing. They involve local testing.

Currently this tool is incompatible with our other tools since it produces versions that are not valid SLS manifest specifications when it could otherwise easily avoid that.

[markelliot]

sls-packaging does, in fact, allow versions that end in ".dirty", allowing you to build local artifacts for local testing. The manifest spec does not allow these versions and shouldn't allow these versions. If you only plan to do local testing, why is this breaking change required? On Mar 13, 2017, at 8:24 PM, Daniel Erenrich <notifications@github.com<mailto:notifications@github.com>> wrote: This ticket is not asking for support. We already have support explicitly called out. See for example test cases in sls-packaging[github.com]<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_palantir_sls-2Dpackaging_blob_338968099c704088ef8d8686fabaf128e0f9839d_sls-2Dversions_src_test_java_com_palantir_slspackaging_versions_SlsProductVersionsTest.java-23L32&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=Vp81aOxWZvuVgVK_wp-VF3pIYG92B19LcCw6XKeYC0U&m=3KihTmQHOgYtaxI840tvZKUYfdJNzkCULLU1t28oZSM&s=TzfMoOT_TINkPKklrMCLsf7VfiDepX4qhepPMM3WS0s&e=>. The spec for the manifest specifically calls out dirty version strings as valid. I have reasons for wanting this. They don't involve publishing. They involve local testing. Currently this tool is incompatible with our other tools since it produces versions that are not valid SLS manifest specifications when it could otherwise easily avoid that. — You are receiving this because you commented. Reply to this email directly, view it on GitHub[github.com]<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_palantir_gradle-2Dgit-2Dversion_pull_50-23issuecomment-2D286311132&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=Vp81aOxWZvuVgVK_wp-VF3pIYG92B19LcCw6XKeYC0U&m=3KihTmQHOgYtaxI840tvZKUYfdJNzkCULLU1t28oZSM&s=eLm64HFFwU_CY9RccgF3PXOSqV7jctTB6-EjsWNI1C8&e=>, or mute the thread[github.com]<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AAp3Q1y49Yv-5F1JCe6IxzrF0VRQls0qrQks5rlghggaJpZM4MbgGD&d=DwMFaQ&c=izlc9mHr637UR4lpLEZLFFS3Vn2UXBrZ4tFb6oOnmz8&r=Vp81aOxWZvuVgVK_wp-VF3pIYG92B19LcCw6XKeYC0U&m=3KihTmQHOgYtaxI840tvZKUYfdJNzkCULLU1t28oZSM&s=REYzqPDSo2WZ9T4Tn9VY6yk9FKB1SmUaqtHnm3KtAxg&e=>.

[derenrich]

I really didn't want to go into this level of detail but ok here's the full story.

You are actually mistaken. sls-packaging allows versions that end in .dirty but only if that is proceeded by a dash. That is per the SLS spec (again there's a ticket where discussion occurred but I can't link to it). If you use this gradle plugin and are exactly on a tag then the version string will be 1.2.3.dirty and so there won't be a dash before the .dirty and so the version is invalid. That is why I am making this change.

Also the manifest spec does allow these version? They are unordered versions which I understand are allowed. They are explicitly mentioned in the spec as valid.

[uschi2000]

Dan, we can change the non-orderable format to something like \d+\.\d+\.\d+(-[a-z0-9-.]*)?(\.dirty)?

[uschi2000]

fyi @derenrich palantir/sls-packaging#202

[derenrich]

Ok but then the sls-packaging gradle plugin will no longer actually be compatible with the actual spec? Or are you proposing changing the SLS spec also?

[uschi2000]

yep, changing the spec too

…

On Tue, Mar 14, 2017 at 3:09 PM Daniel Erenrich ***@***.***> wrote: Ok but then the sls-packaging gradle plugin will no longer actually be compatible with the actual spec? Or are you proposing changing the SLS spec also? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#50 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGOdwTw24w5bKYa35YprzKOYniuTGdgeks5rlq2kgaJpZM4MbgGD> .

[derenrich]

Just FYI. I started this journey at the spec asking it to be changed. It was suggested this was the easier change.

[markelliot]

It's unreasonable that `1.0.0-dirty` is allowable but `1.0.0.dirty` is not. I'd prefer we make the spec more pedantic w/r/t dirty markers instead of break this plugin.

[derenrich]

Ok. Maybe you can convince them better than I.