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

Remove deprecated var.defaults and var.private #140

Closed

Conversation

soerenmartius
Copy link
Member

No description provided.

@soerenmartius soerenmartius requested review from mariux and a team as code owners November 8, 2022 23:50
@soerenmartius soerenmartius marked this pull request as draft November 8, 2022 23:50
Comment on lines +10 to +14
### Removed

- BREAKING CHANGE: Removed deprecated `var.defaults`
- BREAKING CHANGE: Removed deprecated `var.private`, use `var.visibility` instead

Copy link
Member

Choose a reason for hiding this comment

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

no need to remove them already unless we refactor the complete module.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that the module is barely maintainable due to the complexity of defaults. I'd still like to merge this PR, migrate to the new test suite as a next step and then start adding more features requested by the community. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

new feature do not need to be added to the complex defaults.. this module is on the list of complete refactor. There is no need to add breaking changes now.

@soerenmartius soerenmartius force-pushed the soerenmartius/remove-deprecated-variables branch 6 times, most recently from a1f07ac to be924ff Compare December 13, 2022 23:29
@soerenmartius soerenmartius force-pushed the soerenmartius/remove-deprecated-variables branch from be924ff to af7c39f Compare December 14, 2022 00:16
@soerenmartius soerenmartius marked this pull request as ready for review December 14, 2022 00:20
@soerenmartius soerenmartius requested a review from mariux December 14, 2022 00:20
@mariux mariux closed this Jan 2, 2023
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