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

[Merged by Bors] - Add getter for RenderGraph Node uuid #1499

Closed
wants to merge 2 commits into from

Conversation

mtsr
Copy link
Contributor

@mtsr mtsr commented Feb 22, 2021

RenderGraph errors only give the Uuid of the node. So for my graphviz dot based visualization of the RenderGraph I really wanted to show it to the user. I think it makes sense to have it accessible for at least debugging purposes.

@Ratysz Ratysz added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use C-Feature A new feature, making something new possible labels Feb 22, 2021
@Ratysz
Copy link
Contributor

Ratysz commented Feb 22, 2021

Don't forget about cargo fmt.

Unrelated note, wasn't there a fix to reduce the noise in CI?.. It's pretty hard to dig stuff out.

@mtsr
Copy link
Contributor Author

mtsr commented Feb 22, 2021

Thanks. I've been toggling formatting on and off in my editor, because I ran into some projects without rustfmt.toml that were nonetheless completed reformatted by my default config.

@cart
Copy link
Member

cart commented Feb 22, 2021

bors r+

bors bot pushed a commit that referenced this pull request Feb 22, 2021
`RenderGraph` errors only give the `Uuid` of the node. So for my graphviz dot based visualization of the `RenderGraph` I really wanted to show it to the user. I think it makes sense to have it accessible for at least debugging purposes.
@bors bors bot changed the title Add getter for RenderGraph Node uuid [Merged by Bors] - Add getter for RenderGraph Node uuid Feb 22, 2021
@bors bors bot closed this Feb 22, 2021
@CleanCut
Copy link
Member

Unrelated note, wasn't there a fix to reduce the noise in CI?.. It's pretty hard to dig stuff out.

@Ratysz I know I noticed it and planned to reduce the noise, but I don't think I did...until now. 😉

bors bot pushed a commit that referenced this pull request Mar 7, 2021
Silence those [annoying rustfmt config warnings](https://github.com/bevyengine/bevy/pull/1499/checks?check_run_id=1950282111#step:5:66) that happen because we have unstable rustfmt options in `rustfmt.toml`, but we run it in stable on CI.  Thanks to @Ratysz for [calling it out](#1499 (comment)). 😄 

The final approach we settled on was to comment out the unstable options in `rustfmt.toml`.  Those who are using `nightly` may  uncomment the unstable options locally if they wish. Once the options stabilize, we can uncomment them again.

We also decided that instead of fixing the alias, we would remove the alias entirely so that we do not introduce a custom `.cargo/config.toml` that would conflict with users' custom version of the same file. This means that instead of using a `cargo ci` alias you should use `cargo run -p ci` or `cargo run --package ci` instead.

<details><summary>Original Approach (abandoned)</summary>
<p>

_We decided **not** to go this way..._

In my quest to find a portable way to filter out the warnings I switched the library used to execute commands from `xshell` to `duct` (as advised by the `xshell` project itself when you want to do less simple things).  This still uses the "xtask" pattern of using a cargo command alias and a rust project for what would have usually been done with a bash script (on posix), just a different helper library is being used internally.

NOTE 1: Also, thanks to some sleuthing by @DJMcNab we were able to fix the broken cargo alias.  The issue turned out to be that `.cargo/config.toml` was being ignored because of `.gitignore`.

NOTE 2: This is a [known breaking change](#1309 (comment)) for anyone working on bevy who has their own local `.cargo/config.toml`.
</p>
</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants