-
Notifications
You must be signed in to change notification settings - Fork 47
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
Add KingsGraph #409
Add KingsGraph #409
Conversation
Merge branch 'kingsgraph' of https://github.com/finnbuck/Digraphs into kingsgraph
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.
Looks good to me.
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.
Now that we are using individual functions, you will also need documentation (you can just copy the structure from another documentation entry).
…o kingsgraph Bringing local repository up to date with remote.
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.
This looks good, thanks. I made some suggestions along the lines of what I suggested in #416 for the square and triangular grid graphs. However, my changes will depend on that PR being merged first (which should be done soon, probably tomorrow).
…o kingsgraph Bringing local repository up to date with remote.
Just to note that since #416 has been merged into |
…o kingsgraph Bring local repository up to date with remote.
…ular grid graph function.
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.
I just (re?)linked the documentation into the manual and pushed that, along with my two documentation suggestions. So if the CI passes for one final time, I think it's time to merge!
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.
Thanks for that extra change. In my opinion this is good enough to be merged now, so I'll do that.
Of course, if now or at a later point there are further changes that you'd like to make, you're always welcome to make a further pull request 🙂
Thanks for this contribution.
Awesome, thanks for the merge! |
Create a global function for generating a King's Graph based on two parameters (board height and width).