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

Invert public/private default #570

Merged
merged 7 commits into from
Jul 16, 2019
Merged

Invert public/private default #570

merged 7 commits into from
Jul 16, 2019

Conversation

rossberg
Copy link
Contributor

This change makes private the default visibility for object fields. To make up for that, the new syntax is now a separate short-hand record form, where only <id> = <exp> syntax is allowed, and all fields are public. (In a future change I'd like to get rid of the new keyword for this, but that has various nasty grammar implications.)

The tests, especially the produce-exchange, show that this inversion often leads to more verbose code, but I think we agree that it's still worth it. I also noted that the PX code makes a hell lot of things public...

Also, some minor grammar tweaks.

@rossberg rossberg requested a review from crusso July 15, 2019 14:52
design/Syntax.md Outdated Show resolved Hide resolved
design/Syntax.md Outdated Show resolved Hide resolved
design/Syntax.md Outdated Show resolved Hide resolved
Copy link
Contributor

@crusso crusso left a comment

Choose a reason for hiding this comment

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

That must have been fun. EDIT: I noticed Syntax.md doesn't document the current module syntax and inlined (some of) the changes needed).

Did you update the guide? EDIT: I didn't see it in the list of files...

rossberg and others added 3 commits July 16, 2019 15:52
Co-Authored-By: Claudio Russo <claudio@dfinity.org>
@rossberg
Copy link
Contributor Author

Guide updated, PTAL.

@crusso
Copy link
Contributor

crusso commented Jul 16, 2019

LGTM, thanks for fixing the slides too.

@rossberg rossberg merged commit 716d584 into master Jul 16, 2019
@rossberg rossberg deleted the public branch July 16, 2019 16:28
chenyan-dfinity pushed a commit that referenced this pull request Jul 16, 2019
dfinity-bot added a commit that referenced this pull request Aug 28, 2024
## Changelog for candid:
Branch: master
Commits: [dfinity/candid@cfa7b54c...0ed73c14](dfinity/candid@cfa7b54...0ed73c1)

* [`0ed73c14`](dfinity/candid@0ed73c1) Add size hints in order to optimize allocation during deserialization ([dfinity/candid⁠#570](https://togithub.com/dfinity/candid/issues/570))
mergify bot pushed a commit that referenced this pull request Aug 28, 2024
## Changelog for candid:
Branch: master
Commits: [dfinity/candid@cfa7b54c...0ed73c14](dfinity/candid@cfa7b54...0ed73c1)

* [`0ed73c14`](dfinity/candid@0ed73c1) Add size hints in order to optimize allocation during deserialization ([dfinity/candid⁠#570](https://togithub.com/dfinity/candid/issues/570))
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.

2 participants