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

Save strings file path in printers.conf by Bryan Cain + feedback applied #711

Merged

Conversation

zdohnal
Copy link
Member

@zdohnal zdohnal commented Jun 1, 2023

The PR is the fix by Bryan Cain from #710 with feedback applied, so if it is correct, we can follow up with 2.4.3 soon.

@zdohnal zdohnal requested a review from michaelrsweet June 1, 2023 05:56
@zdohnal zdohnal added this to the v2.4.3 milestone Jun 1, 2023
@zdohnal zdohnal added bug Something isn't working priority-high labels Jun 1, 2023
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.

We don't need the strings directive in printers.conf if load_ppd sets it when the file is already present...

@zdohnal zdohnal force-pushed the Plombo-printer-conf-strings branch from 03d71bf to 43b5360 Compare June 1, 2023 13:47
@zdohnal zdohnal force-pushed the Plombo-printer-conf-strings branch from 3c89347 to a88956a Compare June 1, 2023 13:57
@zdohnal zdohnal requested a review from michaelrsweet June 1, 2023 13:58
@zdohnal
Copy link
Member Author

zdohnal commented Jun 1, 2023

Aha, I thought the access() call is needed at the top of the Bryan's PR, not replacing it... I hope it is ok now.

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.

+1

@michaelrsweet michaelrsweet merged commit ed799a2 into OpenPrinting:master Jun 1, 2023
@zdohnal
Copy link
Member Author

zdohnal commented Jun 1, 2023

@michaelrsweet I would go for release 2.4.3 with this merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority-high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants