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

Refactor generate package #442

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

haoming29
Copy link
Contributor

@haoming29 haoming29 commented Dec 1, 2023

Close #304

One question for the reviewer is that do we actually want to exclude the generator files for the release? I tried to use //go:build ignore in the generate package but that will make go generate malfunction with missing function import in between Go files.

@haoming29 haoming29 requested a review from turetske December 1, 2023 17:25
@haoming29 haoming29 added the internal Internal code improvements, not user-facing label Dec 1, 2023
@haoming29 haoming29 added this to the v7.3.0 milestone Dec 1, 2023
@haoming29 haoming29 changed the title Refactor generate package Refactor generate package Dec 1, 2023
@turetske
Copy link
Collaborator

turetske commented Dec 4, 2023

@haoming29 I do think, ideally, in releases we would include the generated files and not the generator files, but I (and Brian) also don't like the idea of committing generated files to the repo which is, I think, what would be required. So I think we need to bow to what works over what's ideal. I'll take a look and see if I have a solution.

@turetske
Copy link
Collaborator

turetske commented Dec 5, 2023

@haoming29 I don't think we want to exclude the generator files in the release, at least for now. I think the solution might be some fussing the with the GitHub ci that allows us to generate the generated files and then exclude the generator files solely in the release pipeline. I also think that's a separate issue than this review.

Copy link
Collaborator

@turetske turetske left a comment

Choose a reason for hiding this comment

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

LGTM!

@haoming29
Copy link
Contributor Author

@haoming29 I don't think we want to exclude the generator files in the release, at least for now. I think the solution might be some fussing the with the GitHub ci that allows us to generate the generated files and then exclude the generator files solely in the release pipeline. I also think that's a separate issue than this review.

Make sense to me. I will create another ticket for what we discussed here

@haoming29 haoming29 merged commit dbfbd41 into PelicanPlatform:main Dec 5, 2023
6 checks passed
@haoming29 haoming29 deleted the refactor-gen-pkg branch December 5, 2023 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Internal code improvements, not user-facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reorganize generate package and go generator
2 participants