-
Notifications
You must be signed in to change notification settings - Fork 110
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
Pimp lua examples (addresses issues #640 #839) #883
base: master
Are you sure you want to change the base?
Conversation
So far this is done as @hmenke suggested, i.e. the libraries are only shown for the first example if multiple examples are given.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know much about Lua parts but looks good to me (except that CI error and a minor comment).
@@ -243,8 +243,9 @@ \subsubsection{Concept: Chain Groups} | |||
|
|||
As can be seen, the placement is not particularly nice by default, use the | |||
algorithms from the graph drawing libraries to get a better layout. For | |||
instance, adding |tree layout| to the above code results in the following | |||
somewhat more pleasing rendering: | |||
instance, adding |tree layout| to the above code (and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this also closes #881 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ilayn, sorry, I thought that I had commented on that already, but it seems that didn't work. So here "again".
You are absolutely right. This is, because I did that change in my forks master branch before following your suggestions to do new changes only in branches. A day after doing these changes I thought that I maybe could have avoided that by checking out the commit before I made that change and creating a branch from there.(?)
I hope that I don't have to do everything again, because this is hopefully a trivial change and therefore doesn't mess up anything when this is applied again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem I'll edit this issue text and they will all be closed automagically once this is merged. I think you fought well with git.
The CI failure is a bug in l3backend. |
I edited the issue description to close the other relevant issues and PRs. Please go over them once if I did the right ones. Then ready when you are ready. |
The mentioned issues and pull requests are right, thank you. While the last two could already be merged, the first one will still take a while I guess (as is also mentioned in the first comment here). |
Closes #640
Closes #755
Closes #881