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

Move more functions to the pdfLetter class #20143

Merged
merged 1 commit into from
Apr 27, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Apr 25, 2021

Overview

Move more functions to the pdfLetter function

Before

Functions on 'common' class that isn't really common and is only used by this class within git universe

After

More functions moved back to the 'real' class. Instances of 'self' replaces to make the process less fraught

Technical Details

This 'common' form isn't really common - it's silly. This PR removes all
the gotcha instances of 'self::' to make it easier to decommission this file

Note that there are no calls to the class outside core in git universe

Unit tests cover the code in question & any fail would be a hard fail

Comments

@civibot
Copy link

civibot bot commented Apr 25, 2021

(Standard links)

This 'common' form isn't really common - it's silly. This PR removes all
the gotcha instances of 'self::' to make it easier to decommission this file

Note that there are no calls to the class outside core in git universe
@eileenmcnaughton eileenmcnaughton changed the title Move more functions to the pdfLetter function Move more functions to the pdfLetter class Apr 26, 2021
@eileenmcnaughton
Copy link
Contributor Author

@colemanw this next one is passing now - it would hard fail on tests if not right

@colemanw
Copy link
Member

Looks good

@colemanw colemanw merged commit 5b90f13 into civicrm:master Apr 27, 2021
@colemanw colemanw deleted the pdf branch April 27, 2021 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants