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

Fix bad Tag name in error message #454

Merged
merged 1 commit into from
Dec 30, 2016
Merged

Conversation

lemoinem
Copy link
Contributor

Was listener instead of subscriber.

Was `listener` instead of `subscriber`.
@lemoinem
Copy link
Contributor Author

lemoinem commented Aug 3, 2015

I'm sorry, did I miss something regarding this PR?

@lemoinem
Copy link
Contributor Author

This PR seems simple enough...

@schmittjoh I understand this is far from critical, but some sort of acknowledgment would be nice. Just to be sure it doesn't slip though the cracks...

@lemoinem
Copy link
Contributor Author

@schmittjoh It's been almost one and a half year... Any news regarding this?

@goetas
Copy link
Collaborator

goetas commented Dec 19, 2016

Hi, sorry for the terribly long feedback loop.

Do you mind adding tests for this?

@goetas goetas self-requested a review December 19, 2016 10:31
@lemoinem
Copy link
Contributor Author

You really want a test for that? It's just a typo or a copy/paste issue in an error message...

I don't even know how I should test a compiler pass.

How about a compromise: If you can provide a basic test case class for the compiler pass to test its basic behavior, I will add a test case for the error message?

@goetas
Copy link
Collaborator

goetas commented Dec 30, 2016

@goetas goetas merged commit aa990f8 into schmittjoh:master Dec 30, 2016
@goetas goetas removed their request for review December 30, 2016 16:31
@lemoinem lemoinem deleted the patch-1 branch January 2, 2017 14:22
@lemoinem
Copy link
Contributor Author

lemoinem commented Jan 2, 2017

Thanks for the reference! And for the merge.

I keep my offer on the table. If you ever make available a basic test for the CP, poke me and I will do my best to provide a PR with the test for this error message. But having a test only for this change, seems useless and (in the context of the PR) overkill. Still, I'm glad we could discuss this.

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