Skip to content

Upgrade to 1.24 #45

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Upgrade to 1.24 #45

wants to merge 1 commit into from

Conversation

eccles
Copy link
Member

@eccles eccles commented Mar 27, 2025 •
edited
Loading

Remove rotten protoc-gen-doc
Upgrade tools version to correspond to those in general codebase.
Use new avid-tools.

Add .gitattributes
Add dependabot.yml

Install protoc-gen-doc binary in ci workflow

Use new go get -tool command to set tools and remove tools.go
Use 'go install tool' to install all tools.
Move all helper go code into separate directory and copy over
when appropriate otherwise installing tools fails with unknown
import.

Testing

The github-action ci.yml executes cleanly and visual inspection of exported files is correct.

AB#10624

@eccles eccles force-pushed the dev/paul/10624-remove-protoc-gen-doc branch 17 times, most recently from f6a0eb7 to 0d593fb Compare March 28, 2025 14:29
@eccles eccles marked this pull request as ready for review March 28, 2025 14:32
@eccles eccles force-pushed the dev/paul/10624-remove-protoc-gen-doc branch 2 times, most recently from b0557a3 to 1a83a03 Compare March 28, 2025 14:59
@eccles eccles marked this pull request as draft March 28, 2025 15:04
@eccles eccles force-pushed the dev/paul/10624-remove-protoc-gen-doc branch 3 times, most recently from 435f658 to 38eadd2 Compare March 28, 2025 15:47
@eccles eccles force-pushed the dev/paul/10624-remove-protoc-gen-doc branch 18 times, most recently from aec25eb to 71018de Compare April 2, 2025 09:37
robinbryce
robinbryce reviewed Apr 2, 2025
View reviewed changes
README.md Outdated
Following this practice removes the need for dual maintenance of dependency versions in the builder image. It also produces a build cycle that is significantly faster.

Cross repository builds in docker while using go.work to refer to locally modified sources don't work. And this setup is essential for an efficient workflow.
1. Execute the `go get -tool` command to add the package to the go.mod file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, where do I do this ? the deleted lines tell me how to do this for existing api support. Do we have taskfile support for doing this at the right spot ?

Copy link
Member Author

Choose a reason for hiding this comment

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

The is a manual process that sets up the go.mod file - it should not be executed in any workflow.

Copy link
Member Author

Choose a reason for hiding this comment

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

if we add a new tool the we would exeute the go get -tool command for that tool once
this changes go.mod - after that tool management is transparent

Copy link
Contributor

Choose a reason for hiding this comment

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

Then I think we need a task rune for that please

robinbryce
robinbryce reviewed Apr 2, 2025
View reviewed changes
README.md

### Use an avid/src/go.work

First point avid at a clone of this repo so that it's imports will resolve
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain how we build against locally changed sources without committing and pushing ?

I don't know if go.work is necessar nor not post this change. But I do want to be able to build forestrie, avid and avid-events against a locally modified common-api.

With this change (and with particular reference to the deleted instructions) how do I do that ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I dont think anything has changed in this regard

I exercised the workflow commands in this README and all just worked.

The only change is how tools are managed not how they are used

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, can you please test that and show some evidence of it in the PR. but sounds good thanks

robinbryce
robinbryce reviewed Apr 2, 2025
View reviewed changes
README.md Outdated
The general use entry points all use Docker and do not support efficiently and
iteratively developing avid and other dependents against a local clone of this
repo.
This repo use the go-task tool and also the go toolchain.
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to explain how go-task tool impacts the work flow

robinbryce
robinbryce reviewed Apr 2, 2025
View reviewed changes
README.md
@@ -68,15 +47,11 @@ If you want to iterate on *just* the helper go code and there tests, do one roun

Then just iterate using `task apis:generate` and `task apis:test`

#### For avid
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume your not saying "we don't support this for avid" rather you are saying it works for any consuming package ? Do I have that right ?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes - the deleted text was written when we only had avid
its any repo now

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#### For avid
#### For any repo

Copy link
Member Author

Choose a reason for hiding this comment

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

will do

@eccles eccles force-pushed the dev/paul/10624-remove-protoc-gen-doc branch from 71018de to 0e4d112 Compare April 2, 2025 10:27
@eccles eccles requested a review from robinbryce April 3, 2025 10:19
@eccles eccles force-pushed the dev/paul/10624-remove-protoc-gen-doc branch from 0e4d112 to 4f72f68 Compare April 3, 2025 13:23
@eccles
Upgrade to 1.24
e53b532 
Remove rotten protoc-gen-doc
Upgrade tools version to correspond to those in general codebase.
Use new avid-tools

Add .gitattributes
Add dependabot.yml

Install protoc-gen-doc binary in ci workflow

Use new go get -tool command to set tools and remove tools.go
Use 'go install tool' to install all tools.
Move all helper go code into separate directory and copy over
when appropriate otherwise installing tools fails with unknown
import.

AB#10624

Signed-off-by: Paul Hewlett <[email protected]>
@eccles eccles force-pushed the dev/paul/10624-remove-protoc-gen-doc branch from 4f72f68 to e53b532 Compare April 22, 2025 16:07
@eccles
Copy link
Member Author

eccles commented Apr 24, 2025

obsolete

@eccles eccles closed this Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waarg waarg Awaiting requested review from waarg

@wp0pw wp0pw Awaiting requested review from wp0pw

@j-hartley j-hartley Awaiting requested review from j-hartley

@landintrees landintrees Awaiting requested review from landintrees

@henry739 henry739 Awaiting requested review from henry739

@robinbryce robinbryce Awaiting requested review from robinbryce

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@eccles @robinbryce