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

add device_uri for found network printers (attempt 2) #402

Closed
wants to merge 8 commits into from
Closed

add device_uri for found network printers (attempt 2) #402

wants to merge 8 commits into from

Conversation

Pro-pra
Copy link

@Pro-pra Pro-pra commented May 27, 2022

from #397

i add "title" and i see tooltip if mouse over printer name.

@zdohnal
Copy link
Member

zdohnal commented May 30, 2022

Hi @Pro-pra ,

thank you for the PR! I've verified that I can see device uri once I have my mouse cursor above the entry for network printers.
It would be great if you squashed your commits in the meantime.

LGTM. @michaelrsweet wdyt?

@zdohnal zdohnal requested review from zdohnal and michaelrsweet May 30, 2022 15:14
Copy link
Member

@zdohnal zdohnal left a comment

Choose a reason for hiding this comment

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

The changes look good to me, but please squash all commits into one.

@Pro-pra
Copy link
Author

Pro-pra commented May 31, 2022

I don't know how to do it now

@zdohnal
Copy link
Member

zdohnal commented May 31, 2022

@Pro-pra you can find SHA ID of the last commit which is not yours (here 411b613 - you can check by git log) and then:

$ git reset --soft 411b6136f450a5
$ git commit -a
$ git push --force

In the commit step you will choose a commit message which covers all the changes you've done.

@Pro-pra
Copy link
Author

Pro-pra commented May 31, 2022

and open new pull request?
I think that a lot of PR is not needed for such a minor problem. Sorry.

@Pro-pra Pro-pra closed this May 31, 2022
@zdohnal
Copy link
Member

zdohnal commented May 31, 2022

No - there's no need for a new PR... You can do it on the same branch you've used for PR.

Copy link
Member

@michaelrsweet michaelrsweet left a comment

Choose a reason for hiding this comment

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

LGTM

@zdohnal zdohnal reopened this Jun 1, 2022
@zdohnal
Copy link
Member

zdohnal commented Jun 1, 2022

The PR pushed as commit 23cef1c . Thank you for the PR!

@zdohnal zdohnal closed this Jun 1, 2022
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.

3 participants