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

Remove deprecated rules #3538

Merged
merged 1 commit into from
Jul 17, 2023
Merged

Conversation

sluongng
Copy link
Contributor

What type of PR is this?

Uncomment one line below and remove others.

Other

What does this PR do? Why is it needed?

These rules should have been removed in 0.39 release but were not.

Which issues(s) does this PR fix?

Fixes #

Other notes for review

@sluongng sluongng force-pushed the sluongng/rm-deprecated branch 3 times, most recently from 4529db6 to f4280f5 Compare April 18, 2023 11:46
Copy link
Member

@fmeum fmeum left a comment

Choose a reason for hiding this comment

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

Since 0.39.0 contained so many fixes and improvements for Bzlmod, we didn't want to tie it to the removals.

I will hold onto merging this until the patch release is out.

Copy link
Contributor

@linzhp linzhp left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, but can you give us a bit more time in removing bindata in Uber? @tyler-french is working on it.

@sluongng
Copy link
Contributor Author

I don't think there is any rush in applying this patch. So if it takes a few days for Uber to sort out the bindata removal, then it should be fine.

Or if you want me to split the bindata removal into a separate PR to merge separately, do let me know.

@linzhp
Copy link
Contributor

linzhp commented Apr 20, 2023

Let's wait and land this, so you don't need to do hair splitting

@sluongng
Copy link
Contributor Author

sluongng commented May 8, 2023

@linzhp any update on this?

@linzhp
Copy link
Contributor

linzhp commented May 8, 2023

@uberzzr is steadily making progress in the internal migration

@linzhp linzhp enabled auto-merge (squash) May 16, 2023 03:40
Copy link
Contributor

@linzhp linzhp left a comment

Choose a reason for hiding this comment

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

We are done with the migration. Thanks for the patience

@sluongng
Copy link
Contributor Author

There seem to be some network flake that affected auto merge here. Please try again 🙏

@fmeum
Copy link
Member

fmeum commented May 16, 2023

@sluongng The link variant of the path test is still failing. Could you send a separate PR to disable it? We know it's unsound.

@sluongng
Copy link
Contributor Author

Yup, reasonable #3559

auto-merge was automatically disabled July 10, 2023 15:05

Head branch was pushed to by a user without write access

@sluongng sluongng force-pushed the sluongng/rm-deprecated branch from 2fb55f9 to fc95a69 Compare July 10, 2023 15:05
@sluongng
Copy link
Contributor Author

@fmeum I rebased this PR on top of master. We should get it merged this time considering the go_path issue has been side stepped. 🤗

@fmeum
Copy link
Member

fmeum commented Jul 10, 2023

@sluongng Thanks for remembering to bring this up again.

@linzhp Should we wait for the release that includes the googleapis changes? We may not want to have too many "breaking" changes in one release.

@linzhp
Copy link
Contributor

linzhp commented Jul 10, 2023

That's my thought too

These rules should have been removed in 0.39 release but were not.
@fmeum fmeum force-pushed the sluongng/rm-deprecated branch from fc95a69 to 48a4b2f Compare July 17, 2023 10:05
@fmeum fmeum enabled auto-merge (squash) July 17, 2023 10:05
@fmeum
Copy link
Member

fmeum commented Jul 17, 2023

@linzhp I will merge this now that 0.41.0 is out.

@fmeum fmeum merged commit b0a9851 into bazel-contrib:master Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants