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: support for readonly class #1046

Merged
merged 5 commits into from Jan 11, 2023

Conversation

genintho
Copy link
Contributor

@genintho genintho commented Dec 7, 2022

Is this approach ok?

According to https://php.watch/versions/8.2/readonly-classes

Abstract classes final classes can also be declared readonly. The order of the keywords does not make a difference.

I know some tests will fail, I am showing this to verify that my approach is ok.

@genintho
Copy link
Contributor Author

genintho commented Dec 8, 2022

@czosel would love your feedback on this approach.

@czosel
Copy link
Collaborator

czosel commented Dec 10, 2022

Hi @genintho - the general approach looks good to me! 👍

@genintho
Copy link
Contributor Author

Not sure why CI is breaking now. Running the typescript command locally show no difference. 🤔 I must be missing something.

implicit version of tsc and @types/nodes conflict
with the global install of typescript done in ci
@genintho
Copy link
Contributor Author

@czosel this is ready for review.

I had to install @types/node to fix CI. In the CI, we globally install the most recent version of typescript, which conflict with the version implicitly installed in the repo.

I would love an alternative solution, but I already waste a couple of hours on this.

@czosel
Copy link
Collaborator

czosel commented Jan 1, 2023

@genintho it seems the typescript issue was due to outdated transitive dependencies (see here for details) - I just fixed it in #1050 by regenerating the lock file. Can you revert the changes to package.json and yarn.lock and rebase on main?

@genintho
Copy link
Contributor Author

genintho commented Jan 8, 2023

@czosel happy new year! 😄

I did what you asked. We should be good to go.

@czosel czosel changed the title Rough draft of support for readonly class feat: support for readonly class Jan 11, 2023
@czosel czosel merged commit 30e229e into glayzzle:main Jan 11, 2023
@czosel
Copy link
Collaborator

czosel commented Jan 11, 2023

@genintho thank you, all the best for you as well! 🎉

LGTM 👍

@czosel
Copy link
Collaborator

czosel commented Jan 11, 2023

Released in v3.1.3 🎉

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