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

Making sure each plugin indicates its default group and feature when … #16728

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

seivan
Copy link

@seivan seivan commented Dec 9, 2024

Objective

  • Updating plugin_group! macro to include the plugin's meta information.
  • Making sure each plugin indicates its default group and feature when applicable.

Solution

  • Output additional cfg flags when using plugin_group!
  • For each plugin, add links to the plugin groups associated and necessary conditions.

Testing

  • Checked that the docs updated accordingly.

…applicable.

Updating `plugin_group!`` macro to include the plugin's meta information.
@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation A-App Bevy apps and plugins labels Dec 10, 2024
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Neat idea, and I like the notes.

Two things:

  • this macro really needs comments to explain how / why it works. I'm very surprised that we can modify docs for the plugin itself!
  • the newly added lines should be below the manually added docs

@alice-i-cecile alice-i-cecile added the S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged label Dec 10, 2024
@seivan
Copy link
Author

seivan commented Dec 10, 2024

  • this macro really needs comments to explain how / why it works. I'm very surprised that we can modify docs for the plugin itself!

That makes two of us, I wasn't expecting it. Maybe you can ask the original developers to add those, I just modified it to include the custom directives from the cfg - in fact, I would have preferred it if we could extract the all(s), not(s) and target(s) to work the same way so they could be highlighted better with markdown instead of just blasted as a straight line. But imho, it's good enough for now and you don't have to check source.

  • the newly added lines should be below the manually added docs

I'll change that.

You want to me remove the macro changes until someone can come along and explain/comment why it works?

@alice-i-cecile
Copy link
Member

Leave the macro changes for now: I think they're an improvement :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-App Bevy apps and plugins C-Docs An addition or correction to our documentation S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants