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

consider enforcing sort order in the build #2368

Closed
pq opened this issue Dec 9, 2020 · 11 comments
Closed

consider enforcing sort order in the build #2368

pq opened this issue Dec 9, 2020 · 11 comments
Assignees
Labels
P3 A lower priority bug or feature request type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable type-task type-test

Comments

@pq
Copy link
Member

pq commented Dec 9, 2020

See churn in #2362. Also, discussion in #2367.

@pq pq added type-task type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable build labels Dec 9, 2020
@srawlins srawlins added the P3 A lower priority bug or feature request label Sep 25, 2022
@pq
Copy link
Member Author

pq commented Sep 1, 2023

Now that we're in the SDK, we can do this easily. 🎉

@pq pq self-assigned this Sep 1, 2023
@pq pq added type-test and removed area-build labels Sep 1, 2023
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Sep 1, 2023
Fixes: dart-lang/linter#2368

Change-Id: I498e271dc32348115a34472b44688be8de2169a3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/323901
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
@pq
Copy link
Member Author

pq commented Sep 1, 2023

Implemented w/ dart-lang/sdk@349e626.

@pq pq closed this as completed Sep 1, 2023
@parlough
Copy link
Member

parlough commented Sep 1, 2023

Nice cleanup! One less thing to spend brain power on 🥳

@pq
Copy link
Member Author

pq commented Sep 1, 2023

💯

@pq
Copy link
Member Author

pq commented Sep 1, 2023

We've wanted this for a while. Nice reward for the long haul of migrating the linter into the SDK!

@parlough
Copy link
Member

parlough commented Sep 1, 2023

I wonder why your commits aren't closing the linked issues automatically? Is it because you're not actually committing, but the Commit Queue is? I've had the same problem with closing site-www issues from the SDK before, but thought it was repo access issues on my end.

@pq
Copy link
Member Author

pq commented Sep 1, 2023

I'm not sure to be honest.

@whesse @athomas?

@athomas
Copy link
Member

athomas commented Sep 2, 2023

I believe it's because copybara (the tool that pushes the code from googlesource.com didn't have push access to the linter repo). I made an attempt at fixing that. Let me know if it works!

Background: to close issues in another repo the user pushing the commit has to have access to the other repo. I couldn't find documentation on what access exactly, though. GitHub scans the pushed commits for keywords and then applies its own rules to decide what to do.

@pq
Copy link
Member Author

pq commented Sep 5, 2023

Thanks for looking!

Not sure it worked unfortunately. I had to make a manual edit here:

#4742

(though I'd expect it to have been fixed in the referencing PR: https://dart-review.googlesource.com/c/sdk/+/323940).

@athomas
Copy link
Member

athomas commented Sep 6, 2023

Not sure it worked unfortunately. I had to make a manual edit here:

Unfortunately, I don't know... 🤷

This is basically what we're doing but I'm not sure what permission we need to grant the bot:
https://docs.github.com/en/repositories/creating-and-managing-repositories/creating-an-issues-only-repository

It's an "app" so I can't grant it write access to linter specifically, and it already has write access to every repo.

@pq
Copy link
Member Author

pq commented Sep 6, 2023

Ah well. Thanks for looking!

On the bright side, the plan is to move the linter issues over to the SDK sooner than later so the issue won't be long lived.

Again, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 A lower priority bug or feature request type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable type-task type-test
Projects
None yet
Development

No branches or pull requests

4 participants