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

Failing tests for 1010 #1072

Conversation

willmurphyscode
Copy link
Contributor

@willmurphyscode willmurphyscode commented Oct 20, 2017

This PR contains failing tests that demonstrate #1010.

I will try to make the tests pass, but I wanted a little input from @kbknapp on how to proceed, and I wanted to make sure @Freyskeyd believes that these tests reproduce the issue he reported correctly.


This change is Reviewable


Changed by @kbknapp to change to reference #1010

@mention-bot
Copy link

@willmurphyscode, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kbknapp to be a potential reviewer.

@Freyskeyd
Copy link

@willmurphyscode Yeah, the code looks good for me.

Are you working on resolving this issue?

@kbknapp
Copy link
Member

kbknapp commented Oct 20, 2017

I think the added tests might be for #1010 whereas #1061 deals with flags specifically which aren't handled in the propagating of values because there is no value to be propagated. To fix #1061 we'd just need to add some logic to propagate whether the flag was used or not and number of occurrences.

I'd recommend renaming this PR to reflect working on #1010 and open a new PR for #1061 to handling propagating of flags.

@Freyskeyd
Copy link

@kbknapp I can open a PR with failing test related to #1061 if you want

@kbknapp
Copy link
Member

kbknapp commented Oct 20, 2017

@Freyskeyd that'd be great thanks!

@kbknapp kbknapp changed the title Failing tests for 1061 Failing tests for 1010 Oct 21, 2017
@kbknapp
Copy link
Member

kbknapp commented Oct 24, 2017

Closing in favor of #1075

@kbknapp kbknapp closed this Oct 24, 2017
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.

4 participants