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

Adding Final to the list of Banking institutions #2085

Merged
merged 9 commits into from Oct 16, 2016
Merged

Adding Final to the list of Banking institutions #2085

merged 9 commits into from Oct 16, 2016

Conversation

arfrank
Copy link
Contributor

@arfrank arfrank commented Oct 7, 2016

2FA stats and logo added

stephengroat
stephengroat suggested changes Oct 7, 2016
View reviewed changes
_data/banking.yml
tfa: No

- name: Final
url: https://www.getFinal.com/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since domains are case insensitive and google return a no www URL, please correct url to https://getfinal.com

Ran through meld, diff showing on Github web ui is wrong

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a doc to publish?

@stephengroat stephengroat added the add site Issue/PR adds a site to the repo. label Oct 7, 2016
@arfrank
Copy link
Contributor Author

arfrank commented Oct 7, 2016 •
edited by stephengroat

Not yet unfortunately. Will add when there is and also update when we move to push and totp too.

@stephengroat
Copy link
Contributor

OK. just make the url change for now and we'll merge

@stephengroat
Copy link
Contributor

Since that security page doesn't mention anything about client facing two factor auth, could you remove it? we can re-add a different page when it's updated with something. thanks!

@arfrank
Copy link
Contributor Author

arfrank commented Oct 8, 2016

Done :)

On Fri, Oct 7, 2016 at 10:07 PM, Stephen notifications@github.com wrote:

Since that security page doesn't mention anything about client facing two
factor auth, could you remove it? we can re-add a different page when it's
updated with something. thanks!

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#2085 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAHzLJEN2dT_VQniHtm_Shg8P6ZO_VKbks5qxxcOgaJpZM4KQwrK
.

stephengroat
stephengroat approved these changes Oct 8, 2016
View reviewed changes
Copy link
Contributor

@stephengroat stephengroat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for all the work!

@stephengroat stephengroat added looks-good merge conflict PR isn't up-to-date with the main branch and must be updated before merging. and removed looks-good labels Oct 12, 2016
@stephengroat
Copy link
Contributor

@arfrank you have a merge conflict. could you resolve it at your convenience?

@arfrank
Copy link
Contributor Author

arfrank commented Oct 15, 2016

@stephengroat Now fixed, not sure why it's seeing whitespace changes, didn't touch anything but our entries, but happy to revert if needed

@Carlgo11 Carlgo11 removed the merge conflict PR isn't up-to-date with the main branch and must be updated before merging. label Oct 15, 2016
@stephengroat stephengroat merged commit 989f756 into 2factorauth:master Oct 16, 2016
bepstein referenced this pull request in theias/supportsso Apr 20, 2018
@arfrank @stephengroat
Adding Final to the list of Banking institutions (#2085)
9d93325 
* Adding Final to banking.yml

* Adding final.png upload

* Fixing via verify.rb

* Fixing final URL and adding security page

* Removing doc for now

* Working on merge conflicts

* Fixing data file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephengroat stephengroat stephengroat approved these changes

@fpigerre fpigerre fpigerre approved these changes

Assignees
No one assigned
Labels
add site Issue/PR adds a site to the repo.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@arfrank @stephengroat @fpigerre @Carlgo11