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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - fix missing paths in ECS SystemParam derive macro v2 #2550

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - fix missing paths in ECS SystemParam derive macro v2 #2550

wants to merge 1 commit into from

Conversation

pbalcer
Copy link
Contributor

@pbalcer pbalcer commented Jul 26, 2021

This is an updated version of #1434 PR. I've encountered this macro problem while trying to use @woubuc's bevy-event-set crate.

@alice-i-cecile
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Jul 26, 2021
@bors
Try #2550:
8f3950b
alice-i-cecile
alice-i-cecile approved these changes Jul 26, 2021
View reviewed changes
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.

Matches the other PR; reasoning makes sense :) Looks good to me, thanks for the contribution!

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior labels Jul 26, 2021
@bors
Copy link
Contributor

bors bot commented Jul 26, 2021

try

Build failed:

@pbalcer
Copy link
Contributor Author

pbalcer commented Jul 26, 2021 •
edited
Loading

This same problem appears to be also present in the main branch. I'm submitting this patch for 0.5.1 since that's what I'm testing with - but is there a process for porting the patches to main?

The android failure appears to be a random fluke - how does one request a rebuild?

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jul 26, 2021 •
edited
Loading

The android failure appears to be a random fluke - how does one request a rebuild?

Bevy org members can ask bors again :) Once your first PR is merged you'll also be able to trigger it with a fresh push. Oh and Cart can manually approve you.

I'm submitting this patch for 0.5.1 since that's what I'm testing with - but is there a process for porting the patches to main?

The branching here is a new process to deal with a minor emergency (a dependency broke semver...); I'm not sure what the right call is here. Previously you'd just target main with this but this style of trivial bug fix is a good candidate for our point releases.

@cart (and @mockersf) what do you think should be done here?

@cart
Copy link
Member

cart commented Jul 26, 2021

what do you think should be done here?

The base branch should be changed to main. Your assessment of the situation is correct. We don't yet have a formal patch release process and we won't accept arbitrary PRs to the 0.5.1 release branch. We are currently discussing how / whether or not to start doing patch releases more aggressively, but until then, all new changes should target main.

@pbalcer pbalcer changed the base branch from release-0.5.1 to main July 27, 2021 17:53
@pbalcer
Copy link
Contributor Author

pbalcer commented Jul 27, 2021

Thanks for the guidance. I've rebased the patch to main.

@pbalcer
Copy link
Contributor Author

pbalcer commented Aug 9, 2021

@alice-i-cecile Sorry to be a bother, but could you please restart the CI? I don't want this tiny fix to get misplaced again.

@mockersf
Copy link
Member

mockersf commented Aug 9, 2021

bors try

bors bot added a commit that referenced this pull request Aug 9, 2021
@bors
Try #2550:
39b213d
@bors
Copy link
Contributor

bors bot commented Aug 9, 2021

try

Build failed:

@mockersf
Copy link
Member

mockersf commented Aug 9, 2021

unrelated error, looks good 👍

@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Aug 9, 2021
@mockersf
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Aug 10, 2021
@pbalcer
fix missing paths in ECS SystemParam derive macro v2 (#2550)
1cb68a2 
This is an updated version of #1434 PR. I've encountered this macro problem while trying to use @woubuc's bevy-event-set crate.

Co-authored-by: Piotr Balcer <piotr@balcer.eu>
@bors
Copy link
Contributor

bors bot commented Aug 10, 2021

Timed out.

@mockersf
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Aug 11, 2021
@pbalcer
fix missing paths in ECS SystemParam derive macro v2 (#2550)
b13472d 
This is an updated version of #1434 PR. I've encountered this macro problem while trying to use @woubuc's bevy-event-set crate.

Co-authored-by: Piotr Balcer <piotr@balcer.eu>
@bors
Copy link
Contributor

bors bot commented Aug 11, 2021

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title fix missing paths in ECS SystemParam derive macro v2 [Merged by Bors] - fix missing paths in ECS SystemParam derive macro v2 Aug 11, 2021
@bors bors bot closed this Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

@mockersf mockersf mockersf approved these changes

Assignees
No one assigned
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@pbalcer @alice-i-cecile @cart @mockersf