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

Bylabel contain #1482

Merged
merged 8 commits into from Feb 11, 2018

Conversation

Projects
None yet
3 participants
@pythonissam
Contributor

pythonissam commented Feb 3, 2018

Before submitting your PR, check that you've:

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

ref: #1480

@MaxGabriel

This comment has been minimized.

Show comment
Hide comment
@MaxGabriel

MaxGabriel Feb 3, 2018

Member

Can you also update the deprecation warnings for the old byLabel to point to byLabelContains for an exact replacement?

Member

MaxGabriel commented Feb 3, 2018

Can you also update the deprecation warnings for the old byLabel to point to byLabelContains for an exact replacement?

@MaxGabriel

This comment has been minimized.

Show comment
Hide comment
@MaxGabriel

MaxGabriel Feb 10, 2018

Member

@pythonissam LGTM. Yesod-test 1.6.1 was released after you started this PR though, can you bump the version you're using in your Cabal file, @since docs, and Changelog?

Member

MaxGabriel commented Feb 10, 2018

@pythonissam LGTM. Yesod-test 1.6.1 was released after you started this PR though, can you bump the version you're using in your Cabal file, @since docs, and Changelog?

@pythonissam

This comment has been minimized.

Show comment
Hide comment
@pythonissam

pythonissam Feb 10, 2018

Contributor

Is this alright? Besides, travis seems to fail. Is this okay?

Contributor

pythonissam commented Feb 10, 2018

Is this alright? Besides, travis seems to fail. Is this okay?

@MaxGabriel

This comment has been minimized.

Show comment
Hide comment
@MaxGabriel

MaxGabriel Feb 10, 2018

Member

@pythonissam Yep that looks good. The Appveyor tests were failing because of a merge conflict with master. It looks like all but one of the Travis tests passed (and the failure was unrelated to your changes). I fixed two merge conflicts in your PR, and I'll let CI run on that before merging.

Member

MaxGabriel commented Feb 10, 2018

@pythonissam Yep that looks good. The Appveyor tests were failing because of a merge conflict with master. It looks like all but one of the Travis tests passed (and the failure was unrelated to your changes). I fixed two merge conflicts in your PR, and I'll let CI run on that before merging.

@pythonissam

This comment has been minimized.

Show comment
Hide comment
@pythonissam

pythonissam Feb 11, 2018

Contributor

I see. Thank you.

Contributor

pythonissam commented Feb 11, 2018

I see. Thank you.

@MaxGabriel MaxGabriel merged commit f2b651b into yesodweb:master Feb 11, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@MaxGabriel

This comment has been minimized.

Show comment
Hide comment
@MaxGabriel

MaxGabriel Feb 11, 2018

Member

Merged, thanks for your work here @pythonissam!

@snoyberg Can you add me as a maintainer of yesod-test on hackage? I couldn't release this

Member

MaxGabriel commented Feb 11, 2018

Merged, thanks for your work here @pythonissam!

@snoyberg Can you add me as a maintainer of yesod-test on hackage? I couldn't release this

@snoyberg

This comment has been minimized.

Show comment
Hide comment
@snoyberg

snoyberg Feb 11, 2018

Member

Done!

Member

snoyberg commented Feb 11, 2018

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment