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.

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

feat: support workspaces for go #864

Merged
merged 22 commits into from
Oct 18, 2024
Merged

feat: support workspaces for go #864

merged 22 commits into from
Oct 18, 2024

Conversation

sergiusens
Copy link
Collaborator

  • Have you signed the CLA?

bowenfan96
bowenfan96 approved these changes Oct 4, 2024
View reviewed changes
Copy link

@bowenfan96 bowenfan96 left a comment

Choose a reason for hiding this comment

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

Thanks very much Sergio! This is just as we discussed and looks great to me with my limited knowledge on craft plugins. I raised a minor point about not supporting existing go.work files inline.

craft_parts/plugins/go_plugin.py Outdated Show resolved Hide resolved
tigarmo
tigarmo requested changes Oct 7, 2024
View reviewed changes
docs/common/craft-parts/reference/plugins/go_plugin.rst Outdated Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/go_plugin.rst Outdated Show resolved Hide resolved
tests/integration/plugins/test_go.py Outdated Show resolved Hide resolved
@sergiusens
feat: support workspaces for go
1e71872 
Signed-off-by: Sergio Schvezov <[email protected]>
@sergiusens
fix: do not consider go.work in VCS
86333f7 
We cannot guarantee consistent results, the build should fail at this
point until we have a mechanism to validate steps.

Signed-off-by: Sergio Schvezov <[email protected]>
@sergiusens
ci: use the go snap instead of the deb
d7d3d81 
Signed-off-by: Sergio Schvezov <[email protected]>
@sergiusens
fix: make go-use a separate plugin
9383e27 
Signed-off-by: Sergio Schvezov <[email protected]>
@sergiusens
fix: sloppy fingers
08cb733 
Signed-off-by: Sergio Schvezov <[email protected]>
@sergiusens sergiusens requested review from tigarmo, bowenfan96 and cmatsuoka and removed request for bowenfan96 October 17, 2024 12:57
@sergiusens
Copy link
Collaborator Author

There is a how do we clean item to deal with, but might need to come later

@sergiusens
fix: linters
d5b1bca 
Signed-off-by: Sergio Schvezov <[email protected]>
@tigarmo
Copy link
Contributor

tigarmo commented Oct 17, 2024

image
😿 😭

@sergiusens
fix: rst references
1d2531f 
Signed-off-by: Sergio Schvezov <[email protected]>
@sergiusens
Copy link
Collaborator Author

@tigarmo sorry about that, there is nothing really left from the original implementation

tigarmo
tigarmo reviewed Oct 17, 2024
View reviewed changes
Copy link
Contributor

@tigarmo tigarmo left a comment

Choose a reason for hiding this comment

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

@sergiusens can you talk about the motivation for moving on from the previous implementation to this one?

craft_parts/plugins/go_use_plugin.py Outdated Show resolved Hide resolved
craft_parts/plugins/go_use_plugin.py Show resolved Hide resolved
craft_parts/plugins/go_plugin.py Outdated Show resolved Hide resolved
@sergiusens
fix: ruff linting
4c0fac1 
Signed-off-by: Sergio Schvezov <[email protected]>
cmatsuoka
cmatsuoka approved these changes Oct 17, 2024
View reviewed changes
Copy link
Collaborator

@cmatsuoka cmatsuoka left a comment

Choose a reason for hiding this comment

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

This seems to do what was discussed. Ordinarily I would call this an experimental plugin but in the use context it would be rather inconvenient to have to plaster --use-experimental-plugins everywhere. It should be fine if we properly address global state handling early next cycle.

craft_parts/plugins/go_use_plugin.py Show resolved Hide resolved
tigarmo
tigarmo approved these changes Oct 18, 2024
View reviewed changes
craft_parts/plugins/go_use_plugin.py Outdated Show resolved Hide resolved
docs/common/craft-parts/reference/plugins/go_use_plugin.rst Outdated Show resolved Hide resolved
sergiusens and others added 2 commits October 18, 2024 14:45
@sergiusens @tigarmo
fix: plugin name in documentation
cf67dae 
Co-authored-by: Tiago Nobrega <[email protected]>
@sergiusens
fix: use public property
86c4791 
Signed-off-by: Sergio Schvezov <[email protected]>
@sergiusens sergiusens merged commit 57cf26f into main Oct 18, 2024
13 checks passed
@sergiusens sergiusens deleted the feature/go-workspaces branch October 18, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmatsuoka cmatsuoka cmatsuoka approved these changes

@tigarmo tigarmo tigarmo approved these changes

@bowenfan96 bowenfan96 bowenfan96 approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@sergiusens @tigarmo @cmatsuoka @bowenfan96