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

Prefer DynamicMemberLookup over KVC #342

Merged
merged 6 commits into from
Aug 28, 2023
Merged

Conversation

art-divin
Copy link
Contributor

@art-divin art-divin commented Aug 20, 2023

Context

On non-Darwin platforms KVC is not supported, thus check in Variable.swift:115 is returning nil.

While this technically cannot be solved, it would be better to first attempt to get value via DynamicMemberLookup protocol conformance.

This change is required for Sourcery linux support: krzysztofzablocki/Sourcery#1188

@art-divin
Copy link
Contributor Author

👋🏻 Hello @djbe ,

I am finishing Linux support for Sourcery.

At the moment this line of code is a blocker, would it be possible to merge this MR anytime soon?

I can see that this repo is unmaintained. I have resources to setup CI here and maybe I'll need to merge some more MRs before Sourcery Linux port would be finished.

I'd love to be collaborator here and support the project 🙏🏻

Copy link
Contributor

@djbe djbe left a comment

Choose a reason for hiding this comment

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

Unfortunately the danger step doesn't work from other repos… Anyway, you're missing a changelog entry.

📝 We use the following format for CHANGELOG entries:

* Prefer `DynamicMemberLookup` over KVC.  
  [##342](https://github.com/stencilproject/Stencil/pull/342)
  [@art-divin](https://github.com/art-divin)

💡 Don't forget to end the line describing your changes by a period and two spaces.

Sources/Stencil/Variable.swift Outdated Show resolved Hide resolved
@djbe djbe merged commit 1aeeced into stencilproject:master Aug 28, 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.

3 participants