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

Update Go Package Index #354

Merged
merged 3 commits into from
Apr 13, 2024

Conversation

MatrixCrawler
Copy link
Collaborator

This possibly fixes #192
We need to add a v in front of the versioning tags though

.github/workflows/publish.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
@MatrixCrawler
Copy link
Collaborator Author

I updated the readme with the version note and the documentation for some missing points.
i reduced the readme file as most of the stuff is covered by the documentation and i think no one really wants to read a five page Readme ;-)
But maybe i am wrong with this.

@crablab
Copy link
Collaborator

crablab commented Apr 12, 2024

I updated the readme with the version note and the documentation for some missing points.
i reduced the readme file as most of the stuff is covered by the documentation and i think no one really wants to read a five page Readme ;-)

Please could you split these changes out into a separate PR? 😅 We now have a couple of PRs with large deltas and which just keep getting longer and longer. That makes it really tricky to review as it's not clear when a PR is "done", and we're ending up in endless cycles of comments as new stuff is being added all the time :)

I think it'd help with review if we can keep them tightly scoped to a particular goal, even if that means we need to come back with more PRs in the future.

@MatrixCrawler
Copy link
Collaborator Author

I updated the readme with the version note and the documentation for some missing points.
i reduced the readme file as most of the stuff is covered by the documentation and i think no one really wants to read a five page Readme ;-)

Please could you split these changes out into a separate PR? 😅 We now have a couple of PRs with large deltas and which just keep getting longer and longer. That makes it really tricky to review as it's not clear when a PR is "done", and we're ending up in endless cycles of comments as new stuff is being added all the time :)

I think it'd help with review if we can keep them tightly scoped to a particular goal, even if that means we need to come back with more PRs in the future.

Moved the readme and documentation update to #376

@yermulnik
Copy link
Collaborator

yermulnik commented Apr 12, 2024

That makes it really tricky to review as it's not clear when a PR is "done", and we're ending up in endless cycles of comments as new stuff is being added all the time :)

+1
force pushes also make it hard to track changes linked from the email notifications.
I'd suggest either to force-push right before the merge, or (preferably) rely upon GH feature to squash and merge when you press the green button to merge PR.
image

example of what I mean about force-pushes:
image
image

@MatrixCrawler
Copy link
Collaborator Author

I will try to rebase less during the review :)

@yermulnik
Copy link
Collaborator

I will try to rebase less during the review :)

Thanks.
Though please take into consideration whet I wrote: there's actually no need to do that in most of the cases in current conditions 😺 The only meaningful time to force-push is right before you call for review.

Copy link
Collaborator

@crablab crablab left a comment

Choose a reason for hiding this comment

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

LGTM - Warren gave a 👍 in the comments, I believe.

Thanks @MatrixCrawler for splitting the PR :)

@MatrixCrawler MatrixCrawler merged commit 4d66b26 into warrensbox:master Apr 13, 2024
@MatrixCrawler MatrixCrawler deleted the publish-to-go-index branch April 13, 2024 18:14
@MatrixCrawler MatrixCrawler added this to the Release 1.1.0 milestone Apr 16, 2024
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.

Update Golang Package Version on pkg.go
4 participants