-
-
Notifications
You must be signed in to change notification settings - Fork 963
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
fix: de-duplicate message IDs #1973
Conversation
clidoc . | ||
.PHONY: docs/cli | ||
docs/cli: | ||
go run ./cmd/clidoc/. . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is way easier to manage rebuilding when needed.
for _, spec := range decl.Specs { | ||
value := spec.(*ast.ValueSpec) // safe because decl.Tok is token.CONST | ||
if t, ok := value.Type.(*ast.Ident); ok { | ||
if t.Name == "ID" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This filters to use const's only of type ID
(i.e. hard-coded).
allFiles = append(allFiles, f) | ||
} | ||
} | ||
_, err = (&types.Config{Importer: importer.Default()}).Check("text", set, allFiles, info) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gave me a bit of a hard time, as I could not get it to properly resolve imports from modules. Therefore, the warning in the id.go
file.
// This file MUST not have any imports to modules that are not in the standard library. | ||
// Otherwise, `make docs/cli` will fail. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this warning
} | ||
allFiles := make([]*ast.File, 0) | ||
for fn, f := range pack.Files { | ||
if strings.HasSuffix(fn, "/id.go") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and hence we only load that file (hard-coded).
Co-authored-by: hackerman <[email protected]>
Related issue(s)
closes #1956
Checklist
introduces a new feature.
contributing code guidelines.
vulnerability. If this pull request addresses a security. vulnerability, I
confirm that I got green light (please contact
[email protected]) from the maintainers to push
the changes.
works.
Further Comments
The clidoc util now ensures that there are no duplicate IDs.
It works, as before changing the ID I got this error: