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

feat(AIP-122): disallow embedded resource #1245

Merged
merged 4 commits into from
Aug 31, 2023

Conversation

noahdietz
Copy link
Collaborator

Disallow embedding resources in other resources, use resource_references instead.

Enforce guidance added in aip-dev/google.aip.dev#1196.

@noahdietz noahdietz requested a review from a team as a code owner August 29, 2023 21:55
docs/rules/0122/embedded-resource.md Show resolved Hide resolved
rules/aip0122/embedded_resource_test.go Outdated Show resolved Hide resolved
## Disabling

If you need to violate this rule, use a leading comment above the method.
Remember to also include an [aip.dev/not-precedent][] comment explaining why.
Copy link
Contributor

Choose a reason for hiding this comment

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

this might be a good place to link to notable exceptions in the AIPs.

Copy link
Contributor

@toumorokoshi toumorokoshi left a comment

Choose a reason for hiding this comment

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

Sorry, forgot that we need to update this to exclude revisions.

rules/aip0122/embedded_resource.go Show resolved Hide resolved
Copy link
Contributor

@toumorokoshi toumorokoshi left a comment

Choose a reason for hiding this comment

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

Great, thanks for doing that!

I think this can be merged even without merging aip-dev/google.aip.dev#1192: no one is using the revisions pattern yet anyway, so the existence of the condition wouldn't impact it's usage as a check now.

@noahdietz
Copy link
Collaborator Author

Great, thanks for doing that!

I think this can be merged even without merging aip-dev/google.aip.dev#1192: no one is using the revisions pattern yet anyway, so the existence of the condition wouldn't impact it's usage as a check now.

I agree! Good idea. :)

@noahdietz noahdietz merged commit e3bbdb3 into googleapis:main Aug 31, 2023
6 checks passed
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.

None yet

2 participants