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

Clean up ns and make code more idiomatic #62

Merged
merged 2 commits into from
Jun 28, 2015
Merged

Clean up ns and make code more idiomatic #62

merged 2 commits into from
Jun 28, 2015

Conversation

danielcompton
Copy link
Contributor

Some of these changes you may not prefer, I'm happy to pull them back out if you'd like.

nbrs)
nil ;not bipartite
;not bipartite
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't quite understand the purpose of this comment.

Copy link
Owner

Choose a reason for hiding this comment

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

In this case, we should leave the if-statement to make it clear that the true branch of the if-statement, which returns nil, indicates that the graph is not bipartite.

@danielcompton
Copy link
Contributor Author

I've signed the Clojure CA and am happy to sign over copyright. I'm a little confused by the idea though, I wouldn't expect signing the Clojure CA would apply to contributions to Loom when it isn't a Core Clojure project?

@aysylu
Copy link
Owner

aysylu commented May 12, 2015

There's no longer a requirement to sign the Clojure CA in order to contribute to Loom. I've announced it some time ago, but it looks like not all the references were updated. The FAQ page should now have the up-to-date information. Sorry for the confusion. The vision behind Loom is to eventually make it a Clojure contrib library, which is why the requirement existed before.

Thank you for your great contributions! I'm still in the process of reviewing the 2 PRs and will get back to you shortly.

@aysylu
Copy link
Owner

aysylu commented Jun 27, 2015

Everything looks good. If you could revert just that one change that we discussed in src/loom/alg.clj on line 437, I'll merge the PR.

@danielcompton
Copy link
Contributor Author

All fixed.

@aysylu
Copy link
Owner

aysylu commented Jun 28, 2015

Great, thanks!

aysylu pushed a commit that referenced this pull request Jun 28, 2015
Clean up ns and make code more idiomatic
@aysylu aysylu merged commit 10aff23 into aysylu:master Jun 28, 2015
@danielcompton danielcompton deleted the clean-up branch June 28, 2015 20:20
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