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

auto bump version in banner.go from previous tag #121

Merged
merged 15 commits into from
Oct 22, 2023

Conversation

dogancanbakir
Copy link
Member

This PR bumps the version in the internal/runner/banner.go from the previous tag. Closes #91.

Copy link
Member

@tarunKoyalwar tarunKoyalwar left a comment

Choose a reason for hiding this comment

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

also to auto bump version in banner while creating release i think we can use set ldflags in gorelease config (ref: https://goreleaser.com/customization/builds/#passing-environment-variables-to-ldflags)

possible implementation

what do you think ??

.github/workflows/autorelease-tag.yml Outdated Show resolved Hide resolved
@dogancanbakir
Copy link
Member Author

@tarunKoyalwar The first two tasks on the list are completed along with additional logical steps. The final task can be utilized to enhance the script.

@tarunKoyalwar tarunKoyalwar requested review from Mzack9999 and removed request for tarunKoyalwar July 6, 2023 15:56
Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

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

I tried testing by forking your fork, but testing becomes quite challenging as GH doesn't let me create a PR. As @tarunKoyalwar pointed out, you probably need to commit after bumping the version in the go file. I think you need to do the following:

  • Add a git commit step after bump
  • Merge your auto_bump_version branch into your main
  • Test creating tag releases and check if the version in the go file matches and gets bumped up

Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

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

To ensure it works, can we try to decrease the autorelease interval in your fork and see if the version is effectively bumped by +1 in Z (vX.Y.Z)?

(I saw some inconsistency in dogancanbakir@011f7ab)

@dogancanbakir
Copy link
Member Author

I did, actually, but on a different branch. That commit was a mistake, so please ignore it. I created a tag 0.1.3 on my fork and ran the workflow and this is the commit it generated dogancanbakir@00b9c79.

Copy link
Member

@tarunKoyalwar tarunKoyalwar left a comment

Choose a reason for hiding this comment

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

testing this PR by forking . not sure about the reason
but version bump is corrupted i.e
changes v1.0.9 -> v..1

tarunKoyalwar@00b4b7b

i think we should have chosen to create a simple go program . with go ast and semver libraries now i think that would have been more stable since semver lib already implements and covers all edgecases related to version handling (cc : @Mzack9999 )

@dogancanbakir
Copy link
Member Author

It sets it to v..1 because it first gets the previous tag and then increases the version. In your case, you needed the tag (tarunKoyalwar@cddcb7e). I agree and -realized now- that it would be better to use a well-tested and maintained tool instead of relying on a custom bash script. I'll try to implement it in go.

Copy link
Member

@Mzack9999 Mzack9999 left a comment

Choose a reason for hiding this comment

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

lgtm! Great suggestion @tarunKoyalwar and implementation @dogancanbakir

Since this looks quite generic and useful what do you think about moving it to the https://github.com/projectdiscovery/utils package under scripts with readme containing instructions?

Example: https://github.com/Mzack9999/cdncheck/actions/runs/5644906529/job/15289638505
Screenshot 2023-07-24 at 14 42 03

@dogancanbakir
Copy link
Member Author

@Mzack9999, Yes, I was thinking the same. I'm on it.

@tarunKoyalwar
Copy link
Member

yeah , i think it is definetly worth moving to utils .
projectdiscovery/utils#218 (review)

we can gather all such useful one use scripts and when we have enough of them we can put all of them in pd-bot (cc: @Mzack9999 )

@dogancanbakir dogancanbakir self-assigned this Jul 28, 2023
Copy link
Member

@ehsandeep ehsandeep left a comment

Choose a reason for hiding this comment

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

Requesting changes to make use of projectdiscovery/utils#218

Copy link
Member

@tarunKoyalwar tarunKoyalwar left a comment

Choose a reason for hiding this comment

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

@ehsandeep ehsandeep closed this Aug 28, 2023
@ehsandeep ehsandeep reopened this Aug 28, 2023
@ehsandeep ehsandeep merged commit b3ae505 into projectdiscovery:main Oct 22, 2023
17 checks passed
@dogancanbakir dogancanbakir deleted the auto_bump_version branch October 23, 2023 08:37
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.

Automation to Update the version to match the release.
5 participants