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

feat: add name and with_count params into listing queues #2018

Merged

Conversation

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0006%) to 79.546% when pulling be62384 on galuszkak:feature/messaging-filter-update into e77d083 on gophercloud:master.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Sep 13, 2020

Build failed.

@galuszkak
Copy link
Contributor Author

I've looked on failing test in acceptance testing but it's related to OAuth. I'm not sure if that error is related to my changes. @jtopjian could you maybe help me understand a little bit how I should read those logs?

@jtopjian
Copy link
Contributor

@galuszkak Sorry - I looked at this yesterday but forgot to comment.

It's not related to this PR, so it can be ignored. I'm quite sure I fixed this test already, so if you wanted to, do a git fetch on the upstream Gophercloud master branch into your fork, rebase your branch, and then force-push. You don't need to do that for this PR, but might be a good idea for future PRs.

@galuszkak
Copy link
Contributor Author

@jtopjian but this is up to date master branch. So I think something is broken on master.

➜  gophercloud git:(feature/messaging-filter-update) git fetch
➜  gophercloud git:(feature/messaging-filter-update) git rebase origin/master
Current branch feature/messaging-filter-update is up to date.
➜  gophercloud git:(feature/messaging-filter-update) git remote -v
galuszkak	git@github.com:galuszkak/gophercloud.git (fetch)
galuszkak	git@github.com:galuszkak/gophercloud.git (push)
origin	git@github.com:gophercloud/gophercloud.git (fetch)
origin	git@github.com:gophercloud/gophercloud.git (push)

@jtopjian
Copy link
Contributor

arg. You're right. I was on an outdated branch when I looked at this.

Yeah, that test in general needs cleaned up. Please ignore it.

@jtopjian
Copy link
Contributor

@galuszkak Ignoring the test failures, is this ready to be reviewed/merged?

@galuszkak
Copy link
Contributor Author

@jtopjian yes. I was concerned only with failing tests.

Copy link
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thank you!

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