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

Fix #1209 #1423

Merged
Merged

Conversation

lassepe
Copy link
Contributor

@lassepe lassepe commented Nov 2, 2021

This moves not_implemented_for to MakieCore to fix #1209.

I'm not sure if the way I imported it in Makie is the correct/preferred way. I failed to understand the pattern that is used to organize the imports.

@lassepe lassepe force-pushed the fix/not_implemented_for_undefined branch from cc50016 to ebe45ce Compare November 2, 2021 09:22
@lassepe
Copy link
Contributor Author

lassepe commented Nov 2, 2021

Also, it may be good to add a test for this but I'd need a pointer where that should go.

@ffreyer
Copy link
Collaborator

ffreyer commented Nov 2, 2021

If that's something you can test after using Makie I'd put it in Makie's tests (i.e. somewhere in Makie.jl/test/)

@lassepe
Copy link
Contributor Author

lassepe commented Nov 2, 2021

The test failures of the docs seem related. Looks like some macro hygene issue; i.e., not_implemented_for seems to be undefined at the callsite of the macro (?).

@SimonDanisch SimonDanisch mentioned this pull request Dec 8, 2021
13 tasks
@SimonDanisch
Copy link
Member

Alright, the docs project just didn't dev MakieCore...

@SimonDanisch SimonDanisch merged commit 751f029 into MakieOrg:master Dec 10, 2021
@lassepe lassepe deleted the fix/not_implemented_for_undefined branch December 10, 2021 21:09
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.

not_implemented_for is not defined where it is used
3 participants