-
Notifications
You must be signed in to change notification settings - Fork 45
Add description line glue property (#481) #483
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
base: master
Are you sure you want to change the base?
Conversation
/** | ||
* @return $this | ||
*/ | ||
public function setTransactionDescriptionLineGlue($transactionDescriptionLineGlue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add type for the argument?
@@ -20,6 +20,14 @@ class SEPAAccount | |||
/** @var string|null */ | |||
protected $blz; | |||
|
|||
/** | |||
* Determines how single lines in transaction descriptions are joined, see #481. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should somehow refer to MT940 / the fact that this is about transaction statements for this account.
@@ -20,6 +20,14 @@ class SEPAAccount | |||
/** @var string|null */ | |||
protected $blz; | |||
|
|||
/** | |||
* Determines how single lines in transaction descriptions are joined, see #481. | |||
* Defaults to an empty string for maximum compatibility but some banks implicitly assume line breaks for this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say "backwards compatibility" (with previous versions of this library) rather than "maximum compatibility" (with the largest number of banks).
* @return $this | ||
*/ | ||
public function setTransactionDescriptionLineGlue($transactionDescriptionLineGlue) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add self
return type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it out to match the other getters in this file.
Please merge it and change it yourself. This is so much faster than back and forth here. |
This PR fixes #481 by adding the optional property
transactionDescriptionLineGlue
to SEPAAccount.