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

[swift] Introduce a ProtoExtensible protocol that all messages that have been extended conform to. #2790

Merged
merged 1 commit into from
Jan 21, 2024

Conversation

dnkoutso
Copy link
Collaborator

This is generally harmless and another tiny step in preparation of a moderate re-design on how Wire proto extensions are generated.

@dnkoutso dnkoutso requested a review from lickel January 19, 2024 18:29
Base automatically changed from unknown_fields_dict to master January 21, 2024 02:13
* See the License for the specific language governing permissions and
* limitations under the License.
*/
public protocol ProtoExtensible {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably have some DocC for why this exists 😉

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@dnkoutso dnkoutso merged commit 8c35841 into master Jan 21, 2024
7 checks passed
@dnkoutso dnkoutso deleted the extensible_protocol branch January 21, 2024 18:07
Comment on lines +53 to +54
val isExtensible: Boolean
get() = extensionsList.isNotEmpty()
Copy link
Member

Choose a reason for hiding this comment

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

What does having extensions make it extensible?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the list of reservations for extending a message, no?

If there are reservations then you can do extend Foo or whatever in proto land.

That's what this is referring to.

Copy link
Member

Choose a reason for hiding this comment

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

Got it, yes.

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

4 participants