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

Misc maintenance #2127

Merged
merged 2 commits into from
Oct 16, 2016
Merged

Misc maintenance #2127

merged 2 commits into from
Oct 16, 2016

Conversation

YesThatAllen
Copy link
Contributor

@YesThatAllen YesThatAllen commented Oct 16, 2016 •
edited
Loading

- Updated steps for getting started with a local version

  • updated urls using https when possible
  • minor spacing cleanup

@YesThatAllen
Copy link
Contributor Author

Seems like the spacing changes in this PR may go against this rubocop PR. Good thing I got it in before that went to master!

@Carlgo11
Copy link
Member

Except I just merged it

@Carlgo11 Carlgo11 added the merge conflict PR isn't up-to-date with the main branch and must be updated before merging. label Oct 16, 2016
@YesThatAllen
Copy link
Contributor Author

Ha ha ha. Happy to rebase if the rest of the commits are desired.

@Carlgo11
Copy link
Member

@YesThatAllen I made some changes to the clean-up to honour our .editorconfig rules.
I also removed 6cc5917 as I'm not as certain on how good that commit turned out.
Maybe commit that in a separate PR?

@Carlgo11 Carlgo11 added enhancement Issue/PR contains enhancements to the overall code of the site. and removed merge conflict PR isn't up-to-date with the main branch and must be updated before merging. labels Oct 16, 2016
@Jawshy Jawshy added the merge conflict PR isn't up-to-date with the main branch and must be updated before merging. label Oct 16, 2016
@Jawshy
Copy link
Contributor

Jawshy commented Oct 16, 2016

Oh noes. Looks like a recent merge has created a new merge conflict. @YesThatAllen, could you please resolve the conflict?

@YesThatAllen
Copy link
Contributor Author

YesThatAllen commented Oct 16, 2016 •
edited
Loading

This all started, fwiw, when my text editor wouldn't save this line in the pastebin entry without forcibly removing that one trailing space. Not sure what was up with that.

Carlgo11
Carlgo11 reviewed Oct 16, 2016
View reviewed changes
_config.yml
@@ -6,7 +6,7 @@ author: Josh Davis
description:
List of sites with Two Factor Auth support which includes SMS, email,
phone calls, hardware, and software.
url: http:https://twofactorauth.org
url: https:https://twofactorauth.org
Copy link
Member

Choose a reason for hiding this comment

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

We have to use HTTP on this line as it otherwise won't let users with incompatible browsers access our site without HTTPS.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should a comment be appended so that anyone else that stumbles upon this will know why HTTP - not HTTPS - is used?

@YesThatAllen
Copy link
Contributor Author

No worries, cleaned up the set of commits while I was at it.

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

Not sure what was up with that.

Ah, til that my editor supports http:https://editorconfig.org/ out of the box.

@fpigerre fpigerre merged commit 28e4164 into 2factorauth:master Oct 16, 2016
@YesThatAllen YesThatAllen deleted the misc-maintenance branch October 17, 2016 03:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Carlgo11 Carlgo11 Carlgo11 left review comments

@Jawshy Jawshy Jawshy left review comments

Assignees
No one assigned
Labels
enhancement Issue/PR contains enhancements to the overall code of the site.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@YesThatAllen @Carlgo11 @Jawshy @fpigerre @watchmansupport