-
Notifications
You must be signed in to change notification settings - Fork 326
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
Groups in DocTags #7337
Groups in DocTags #7337
Conversation
3347b55
to
6e8b1ac
Compare
a284977
to
acb24e0
Compare
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.
Does this solution also renders further work #6497 unnecessary? Oh, my procrastination and laziness...
Co-authored-by: Ilya Bogdanov <[email protected]>
Co-authored-by: Ilya Bogdanov <[email protected]>
I believe the longer term approach is to go for attributes still. |
Yep, our plans is to use doc tags instead of attributes before the support for attributes lands. |
QA: 🟢 |
@jdunkerley @radeusgd @GregoryTravis I need an accept from one of you, as I changed libraries' code. |
- Input: {} | ||
- Web: {} |
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.
Do the groups still need to be defined in package.yaml
?
What if we annotate a method with a GROUP xyz
when xyz
is not defined in package.yaml
?
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.
Yes the groups have to be defined in the package.yaml.
It is supposed to generate an error but that may not be done yet,
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.
IDE will print the error log and the entry will not be assigned to any group.
Fixes #7336 in a quick way. Next to the old way of defining groups, the library can just add `GROUP` tag to some entities, and it will be added to the group specified in tag's description. The group name may be qualified (with project name, like `Standard.Base.Input/Output`) or just name - in the latter case, IDE will assume a group defined in the same library as the entity. Also moved some entities from "export" list in package.yaml to GROUP tag to give an example. I didn't move all of those, as I assume the library team will reorganize those groups anyway. @jdunkerley @radeusgd @GregoryTravis When you will start specifying groups in tags, remember that: * The groups still belongs to a concrete project; if some entity outside a project wants to be added to its group, the "qualified" name should be specified. See `Table.new` example in this PR. * If the group name does not reflect any group in package.yaml **the tag is ignored**. * A single entity may be only in a single group. If it's specified in both package.yaml and in tag, the tag takes precedence. --------- Co-authored-by: Ilya Bogdanov <[email protected]>
Pull Request Description
Fixes #7336 in a quick way.
Next to the old way of defining groups, the library can just add
GROUP
tag to some entities, and it will be added to the group specified in tag's description.The group name may be qualified (with project name, like
Standard.Base.Input/Output
) or just name - in the latter case, IDE will assume a group defined in the same library as the entity.Also moved some entities from "export" list in package.yaml to GROUP tag to give an example. I didn't move all of those, as I assume the library team will reorganize those groups anyway.
Important Notes
@jdunkerley @radeusgd @GregoryTravis When you will start specifying groups in tags, remember that:
Table.new
example in this PR.Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build
.