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: allow persistent? actor to switch default and transient as synonym for flexible #4786

Merged
merged 30 commits into from
Dec 5, 2024

Conversation

crusso
Copy link
Contributor

@crusso crusso commented Nov 29, 2024

This is the simplest solution with least grammar irregularities and probably least required doc changes.

  • adds keywords persistent and transient.
  • transient is a synonym for flexible.
  • persistent? actor (class) ... flips the default from flexible to stable.

Parses nicely because there is no any longer ambiguity between stable/flexible uses as object modifier or visibility. That means we don't need fancy and confusing grammar factoring.

@crusso crusso requested review from luc-blaeser and ggreif November 29, 2024 15:17
@crusso crusso marked this pull request as ready for review November 29, 2024 15:59
Copy link

github-actions bot commented Nov 29, 2024

Comparing from 6603c0a to 08cb25a:
The produced WebAssembly code seems to be completely unchanged.

@luc-blaeser
Copy link
Contributor

Thank you for this PR!

@crusso
Copy link
Contributor Author

crusso commented Dec 2, 2024

Nice and simple PR. Let's get this merged! Just two things that we can address in follow-up PRs:

  1. We should also streamline documentation to this syntax. I'll ask @jessiemongeon1
  2. We could deprecate the old actor syntax with warning in a next step. Or ask users to explicitly specify stability modifier on old actor fields.

Maybe let me take a pass first before getting @jessiemongeon1 involved. She has tons on her plate and it might be quicker for me to revise the relevant bits.

luc-blaeser
luc-blaeser previously approved these changes Dec 3, 2024
Copy link
Contributor

@luc-blaeser luc-blaeser left a comment

Choose a reason for hiding this comment

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

Nice! Let's merge this.

crusso and others added 2 commits December 3, 2024 15:21
* adapt manual

* upgrade examples

* revert change to intro examples

* revert Counter.mo

* tweaks

* more tweaks

* more tweaks

* more tweaks

* more

* fix message-inspection linenumbers

* more tweaks

* Update doc/md/reference/language-manual.md

* Update doc/md/writing-motoko/writing-intro.md

* Update doc/md/canister-maintenance/upgrades.md

Co-authored-by: Gabor Greif <[email protected]>

* Update doc/md/examples/CardShuffle.mo

Co-authored-by: Gabor Greif <[email protected]>

* Update doc/md/examples/PersistentCounter.mo

Co-authored-by: Gabor Greif <[email protected]>

* Update doc/md/examples/RawRand.mo

Co-authored-by: Gabor Greif <[email protected]>

* Apply suggestions from code review

Co-authored-by: Gabor Greif <[email protected]>

* Update doc/md/canister-maintenance/compatibility.md

Co-authored-by: Jessie Mongeon <[email protected]>

* Update doc/md/canister-maintenance/upgrades.md

Co-authored-by: Jessie Mongeon <[email protected]>

* Update doc/md/canister-maintenance/upgrades.md

Co-authored-by: Jessie Mongeon <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jessie Mongeon <[email protected]>

* Apply suggestions from code review

Co-authored-by: Jessie Mongeon <[email protected]>

---------

Co-authored-by: Gabor Greif <[email protected]>
Co-authored-by: Jessie Mongeon <[email protected]>
@crusso crusso changed the title experiment: allow persistent? actor to switch default and transient as synonym for flexible feat: allow persistent? actor to switch default and transient as synonym for flexible Dec 3, 2024
@ggreif ggreif changed the title feat: allow persistent? actor to switch default and transient as synonym for flexible feat: allow persistent? actor to switch default and transient as synonym for flexible Dec 3, 2024
@crusso crusso requested a review from luc-blaeser December 4, 2024 09:43
luc-blaeser
luc-blaeser previously approved these changes Dec 4, 2024
Copy link
Contributor

@luc-blaeser luc-blaeser left a comment

Choose a reason for hiding this comment

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

Great doc overhaul!

ggreif
ggreif previously approved these changes Dec 4, 2024
Copy link
Contributor

@ggreif ggreif left a comment

Choose a reason for hiding this comment

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

Two minor things, otherwise LGTM!

@crusso crusso dismissed stale reviews from ggreif and luc-blaeser via 08cb25a December 5, 2024 13:44
@crusso crusso added the automerge-squash When ready, merge (using squash) label Dec 5, 2024
@ggreif
Copy link
Contributor

ggreif commented Dec 5, 2024

@Mergifyio update

Copy link
Contributor

mergify bot commented Dec 5, 2024

update

☑️ Nothing to do

  • #commits-behind > 0 [📌 update requirement]
  • -closed [📌 update requirement]
  • -conflict [📌 update requirement]
  • queue-position = -1 [📌 update requirement]

@ggreif ggreif merged commit c606c75 into master Dec 5, 2024
8 checks passed
@ggreif ggreif deleted the claudio/stable-default-progdec-keywords-simp branch December 5, 2024 14:10
@mergify mergify bot removed the automerge-squash When ready, merge (using squash) label Dec 5, 2024
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