-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 Caisse Epargne bank (FR) #2977
Conversation
Add Caisse Epargne Bank
Add Caisse Epargne Bank (FR)
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.
Thanks for the pull request, @chatainsim. Unfortunately, there are a couple of issues that I'm hoping you could resolve. Please resize the image to exactly 32 by 32 pixels and under 2,500 bytes in size. Saving the image at a bit depth of 8 rather than 32 can help reduce the file size.
Reducing size of Caisse d'Epargne icon
Done |
I think the Twitter account should be @CaissEpargneSAV (I know, an 'e' is missing at character 6) rather than @Caisse_Epargne. The first one is for asking questions (Nous répondons à vos questions). The second one is for marketing purposes (Découvrez l'actualité de la marque Caisse d'Epargne à travers ses multiples engagements !). See my (submitted too quickly) PR. |
@nagromc yes it's better I think. I didn't know that they have a 'SAV' account. Good to know. |
Change twitter account
Without white border. Thanks @Carlgo11
@Carlgo11 thanks, updated |
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.
Looking good! 👍
_data/banking.yml
Outdated
@@ -163,6 +163,14 @@ websites: | |||
phone: Yes | |||
doc: https://www.mybankofinternet.com/tob/live/usp-core/static/help.html#login_security | |||
|
|||
- name: Caisse D'Eparnge |
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.
On their Facebook and Twitter page they've named themselves Caisse d'Epargne with a lowercase d. I don't speak French so I won't try to request any changes here but if it is a typo here you might want to change that. 😉
If not then this PR is good to go.
Change D to lower case for D'Epargne
Updated 👍 Thanks |
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.
There is a typo: "Epargne"
Fixed |
Now #2977 (comment) is reverted. :/ |
@Carlgo11 what does that mean? |
@chatainsim the |
I'm not able to find any capital D, sorry. |
I agree. See the line 166 of modified file in the PR. I don't see the problem @stephengroat @Carlgo11 |
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.
thanks for the pr @chatainsim
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.
#2977 (comment)
Right. And I asked you to change it to uppercase.
Edit: I fixed it in 14c2868
This reverts commit fa6b409.
Thanks, I was reading too quickly. My bad. |
@Carlgo11 According to convention, the capital letter in this sentence is already the "É" in Épargne. Capitalising the "d" is unnecessary. This is shown on the Caisse d'Épargne Facebook page as well. |
Add Caisse Epargne bank (FR)