Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare v2.0.0 #40

Merged
merged 1 commit into from
Jul 15, 2020
Merged

Prepare v2.0.0 #40

merged 1 commit into from
Jul 15, 2020

Conversation

SVilgelm
Copy link
Member

Changed the minimum version, In order to fix the compatibility issues of v1.2.2 action and v1.28.3 lint.
Add compatibility section in README.md

Fixes #39 #37

After merging this PR we need to raise 1.2.3 release and remove 1.2.2 release (deprecate it)

@SVilgelm SVilgelm linked an issue Jul 14, 2020 that may be closed by this pull request
@SVilgelm SVilgelm requested a review from a team July 14, 2020 13:27
@SVilgelm
Copy link
Member Author

@sayboras any thoughts?

README.md Outdated
@@ -7,6 +7,12 @@ The action runs [golangci-lint](https://github.com/golangci/golangci-lint) and r

![GitHub Annotations](./static/annotations.png)

## Compatibility

* `v1.2.3` works with `golangci-lint` version >= `v1.28.3`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this is still considered as semantic versioning.

The change we made in 1.2.2 is considered as breaking, so having restriction on patch version (e.g. v1.2.3) is not considered in my opinion as backward compatible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is 1.3.0 good option?
Or we should go with 2.0.0?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally think 2.0.0 beter. Not sure what others think.

@sayboras
Copy link
Member

@sayboras any thoughts?

@SVilgelm just want to boucing idea here. One alternative to fix this one, and make release backward compatible for patch version is to check golangci-lint version, and decide if we need to append the path-prefix or not. What do you think ?

@sayboras sayboras requested a review from a team July 14, 2020 22:55
@SVilgelm
Copy link
Member Author

@sayboras hmmm, I would prefer to release v2 instead of adding such logic, I don't see any problems with removing 1.2.2 release and releasing it as 2.0.0
so someone will need to get back to v1.2.1, someone will just upgrade to v2

My concern here is if we will add such logic once, then what will stop us to do this again and again? and then we will have lots of if..else blocks

let's do it right now

@sayboras
Copy link
Member

@sayboras hmmm, I would prefer to release v2 instead of adding such logic, I don't see any problems with removing 1.2.2 release and releasing it as 2.0.0
so someone will need to get back to v1.2.1, someone will just upgrade to v2

My concern here is if we will add such logic once, then what will stop us to do this again and again? and then we will have lots of if..else blocks

let's do it right now

Yeah, I don't like that a lot of if else either, makes the code ugly to read and maintain.

v2.0.0 sounds good to me. But will v1.x still be under support? golangci-lint supports last two releases I think

@SVilgelm SVilgelm changed the title Prepare v1.2.3 Prepare v2.0.0 Jul 15, 2020
@SVilgelm SVilgelm force-pushed the v1.2.3 branch 2 times, most recently from 47cbfc1 to aec388c Compare July 15, 2020 03:00
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 💯

@sayboras sayboras requested review from a team July 15, 2020 10:06
@denis-tingaikin
Copy link
Member

Also, this PR closes this #38 :)

@SVilgelm
Copy link
Member Author

Also, this PR closes this #38 :)

It's not closes, using a patch part of version nis still not allowed, or I did not understand the problem :)

@SVilgelm SVilgelm merged commit c238b72 into master Jul 15, 2020
@SVilgelm SVilgelm deleted the v1.2.3 branch July 15, 2020 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v1.2.2 was broke if using working-directory Add a warning when using too old golangci version
3 participants