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.

Sign up for GitHub

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

Jump to bottom

Prawn update #3148

Merged
merged 4 commits into from
Mar 3, 2023
Merged

Prawn update #3148

merged 4 commits into from
Mar 3, 2023

Conversation

akostadinov
Copy link
Contributor

also a lot of fixes and improvements
reduce dependencies

can be merged once SaaS OCP migration is finished

@akostadinov akostadinov self-assigned this Jan 3, 2023
@jlledom
Copy link
Contributor

jlledom commented Jan 11, 2023

I added this to the list of things to do after migrating to OCP

jlledom
jlledom reviewed Jan 11, 2023
View reviewed changes
Gemfile Show resolved Hide resolved
Gemfile Show resolved Hide resolved
@akostadinov akostadinov mentioned this pull request Jan 18, 2023
Merged
jlledom
jlledom previously approved these changes Jan 20, 2023
View reviewed changes
@github-actions github-actions bot added the Stale label Feb 4, 2023
@akostadinov akostadinov removed the Stale label Feb 6, 2023
@3scale 3scale deleted a comment from github-actions bot Feb 6, 2023
@github-actions github-actions bot added the Stale label Feb 21, 2023
@3scale 3scale deleted a comment from github-actions bot Feb 21, 2023
@akostadinov akostadinov removed the Stale label Feb 21, 2023
@akostadinov akostadinov changed the title [DO NOT MERGE] Prawn update Prawn update Feb 28, 2023
@akostadinov
Copy link
Contributor Author

Codeclimate can't show results for some reason. I think whatever is saw is mostly because of refactoring things that were similar in the past. So I don't want to do much more refactoring than what I already did and will merge as long as tests pass.

@akostadinov akostadinov force-pushed the prawn branch 10 times, most recently from a666ae5 to 4670db9 Compare March 1, 2023 23:28
@akostadinov
Copy link
Contributor Author

akostadinov commented Mar 1, 2023 •
edited
Loading

@jlledom I had to make some fixes to invoice. Not sure how I missed in the past. But this is previous and new invoice:
new.pdf
prev.pdf

In summary adding table headers, working BOLD text, Liberation font and layout is more dynamic than before, potentially avoiding issues with longer entries.

Please review again. You can skip the licenses commits as I had to do these to get CI pass. Of course feedback on them also welcome.

@akostadinov akostadinov requested a review from jlledom March 1, 2023 23:31
jlledom
jlledom previously approved these changes Mar 2, 2023
View reviewed changes
app/lib/pdf/finance/invoice_generator.rb Outdated Show resolved Hide resolved
app/lib/pdf/printer.rb Outdated Show resolved Hide resolved
@akostadinov akostadinov mentioned this pull request Mar 2, 2023
Merged
@jlledom jlledom mentioned this pull request Mar 2, 2023
Merged
@jlledom jlledom mentioned this pull request Mar 2, 2023
Merged
@akostadinov akostadinov requested a review from jlledom March 2, 2023 19:15
@akostadinov
Copy link
Contributor Author

@jlledom unless you find somehting else bad, I thin kwe can merge now

@akostadinov akostadinov merged commit 11a06dc into 3scale:master Mar 3, 2023
@akostadinov akostadinov mentioned this pull request Mar 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlledom jlledom jlledom approved these changes

Assignees

@akostadinov akostadinov

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@akostadinov @jlledom @hallelujah