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

combinatorics coverage #11804

Merged
merged 1 commit into from
Jun 23, 2015
Merged

combinatorics coverage #11804

merged 1 commit into from
Jun 23, 2015

Conversation

Ismael-VC
Copy link
Contributor

Test next for zero length permutation.

@nalimilan
Copy link
Member

Thanks. Wouldn't it be more logical to use collect(permutations([])), though?

@tkelman
Copy link
Contributor

tkelman commented Jun 21, 2015

@nalimilan could you elaborate? how about testing both?

@nalimilan
Copy link
Member

Why not testing both, but next is tested by collect AFAIK, and the current code depends on the implementation detail of what the iterator state is. We don't really care if it changes, as long as iteration (and thus collect) still works.

@Ismael-VC
Copy link
Contributor Author

@nalimilan I will change it according to your suggestions.

Today I will try to set up my fork of Julia to work with Travis/CoverAlls. Since right now I don't get "immediate" feedback of whether a test actually covered some code-path or if it didn't. It seems that the Julia build-bots used for coverage are only run once or twice a day.

That will make this quest easier!

@Ismael-VC
Copy link
Contributor Author

Another option is to learn to use the "manual"command line coverage tools, but for the moment I'd rather use CoverAlls, so I can keep focusing in learning git and Julia instead of yet another command line tool.

@Ismael-VC
Copy link
Contributor Author

@nalimilan Is this correct?

@nalimilan
Copy link
Member

Looks good to me. AppVeyor failure is unrelated, I don't think it's worth restarting it.

@tkelman Is it fine with you?

@nalimilan
Copy link
Member

Oh, and could you squash the commits?

@Ismael-VC
Copy link
Contributor Author

@nalimilan squashing done!

@tkelman
Copy link
Contributor

tkelman commented Jun 23, 2015

cancelled appveyor, it will probably time out due to #11818 - we can restart the build later if that gets fixed, though there's likely to be a worse-than-usual backlog for the next few days

@nalimilan
Copy link
Member

I would just merge it, I don't see why it would fail on AppVeyor and not on Travis.

tkelman added a commit that referenced this pull request Jun 23, 2015
@tkelman tkelman merged commit 5651bba into JuliaLang:master Jun 23, 2015
@Ismael-VC Ismael-VC deleted the combinatorics-coverage branch June 23, 2015 17:59
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