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

Adding diesel to the cargotest suite #81507

Merged
merged 1 commit into from
Apr 3, 2021
Merged

Adding diesel to the cargotest suite #81507

merged 1 commit into from
Apr 3, 2021

Conversation

weiznich
Copy link
Contributor

As discussed in #79560 (comment) this adds diesel to the compilers test suite. This is basically a reopened version of #79599, but now with the backing of the compiler team.

r? @pnkfelix

@weiznich
Adding diesel to the cargetest suite
99a33b3 
This was proposed in rust-lang#79459 and rust-lang#79560.
Diesel stresses the trait system implementation quite a lot and there
have been various regressions regarding that in the past.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 29, 2021
@pnkfelix pnkfelix mentioned this pull request Feb 2, 2021
Closed
@pnkfelix pnkfelix added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 2, 2021
@pnkfelix
Copy link
Member

pnkfelix commented Feb 2, 2021

Nominating for discussion at T-compiler meeting. Potentially needs to be tagged with T-infra as well; we can discuss that there.

This is a reopened version of #79599, which had been closed back in december with the following comment:

I don't think we should be expanding the set of crates in the cargotest suite until we have a T-compiler/T-infra FCP'd policy about how additions should get reviewed (e.g., who approves? how?). So I'm going to close this for now, but I'd love to see such a policy written; the first step would be to draft something reasonable and file an MCP to discuss that I think.

I am not sure I want to let landing this have to be blocked on coming up with a more general MCP and getting it approved. Or at least, I would the compiler team to at least consider the idea of approving landing this without requiring the construction and approval of a more general policy.

@Mark-Simulacrum
Copy link
Member

I am not necessarily opposed to landing, but I would like some temporary policy at least - it can be as simple as "T-compiler and T-infra FCP" for example.

@apiraino apiraino added T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. and removed I-nominated labels Feb 4, 2021
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 23, 2021
@weiznich
Copy link
Contributor Author

@pnkfelix @Mark-Simulacrum Is there any chance to get this going some time soon?

@pnkfelix
Copy link
Member

I think we should move forward with this. I'm sorry I let it sit so long.

Lets go ahead and fire off an FCP to the tagged teams.

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Feb 26, 2021 •
edited by nikomatsakis
Loading

Team member @pnkfelix has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Feb 26, 2021
@kennytm kennytm changed the title Adding diesel to the cargetest suite Adding diesel to the cargotest suite Feb 26, 2021
@petrochenkov
Copy link
Contributor

If T-infra is ok with this, then I'm ok with this too.

@weiznich
Copy link
Contributor Author

@Mark-Simulacrum @pnkfelix Is there anything we can do to move this forward?

@Mark-Simulacrum
Copy link
Member

@nikomatsakis @nagisa or @eddyb I think need to tick their boxes (and maybe @aidanhs ); I'll try to include a reminder in compiler team meeting this week and I suspect we'll be able to move forward after that. Sorry it's been such a long wait!

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Mar 24, 2021
@rfcbot
Copy link

rfcbot commented Mar 24, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Mar 24, 2021
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Apr 3, 2021
@rfcbot
Copy link

rfcbot commented Apr 3, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Apr 3, 2021
@Mark-Simulacrum
Copy link
Member

Thanks for your patience, @weiznich! @bors r+

@bors
Copy link
Contributor

bors commented Apr 3, 2021

📌 Commit 99a33b3 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 3, 2021
@bors
Copy link
Contributor

bors commented Apr 3, 2021

⌛ Testing commit 99a33b3 with merge 0b417ab...

@bors
Copy link
Contributor

bors commented Apr 3, 2021

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 0b417ab to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 3, 2021
@bors bors merged commit 0b417ab into rust-lang:master Apr 3, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 3, 2021
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pickfire pickfire pickfire left review comments

@pnkfelix pnkfelix pnkfelix left review comments

Assignees

@pnkfelix pnkfelix

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.53.0
Development

Successfully merging this pull request may close these issues.