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

Remove unnecessary conversions #357

Merged
merged 1 commit into from Aug 19, 2019
Merged

Remove unnecessary conversions #357

merged 1 commit into from Aug 19, 2019

Conversation

muesli
Copy link
Contributor

@muesli muesli commented Jul 19, 2019

sample_01 is already a byte slice. Those conversions don't make any difference.

sample_01 is already a byte slice. Those conversions don't make any difference.
@williammartin
Copy link
Sponsor Collaborator

I agree with you, but it seems like we should just remove this test. The assertion is already made in other tests.

@blgm
Copy link
Collaborator

blgm commented Aug 19, 2019

@muesli, thank you for spotting this problem. Because we haven't heard from you in a while I'm going to merge this PR so that you get GitHub contributor credit, and then immediately delete the test as suggested by @williammartin above to remove duplication. Thank you for you helping to make Gomega better!

@blgm blgm merged commit 7bf756a into onsi:master Aug 19, 2019
blgm added a commit that referenced this pull request Aug 19, 2019
- `sample_01` is already a byte slice, so casting it makes no
differrence
- there is already an identity test for `sample_01`
- see #357 for context
@muesli
Copy link
Contributor Author

muesli commented Aug 19, 2019

@blgm Thank you, that's very kind! Also I'm sorry, I didn't realize you were waiting for my action or feedback. What @williammartin mentioned sounded absolutely reasonable, I was just expecting other to chime in or a clear signal how to progress with this PR. Thanks for resolving it!

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.

None yet

3 participants