Skip to content

Accessibility Fixes Round 2#468

Merged
miketaylr merged 1 commit intowebcompat:masterfrom
andrewmcgivery:master
Dec 11, 2014
Merged

Accessibility Fixes Round 2#468
miketaylr merged 1 commit intowebcompat:masterfrom
andrewmcgivery:master

Conversation

@andrewmcgivery
Copy link
Copy Markdown
Contributor

Remainder (Hopefully) of Accessibility fixes.

Closes: #464

@andrewmcgivery andrewmcgivery changed the title Accessibility Fixes Found 2 Accessibility Fixes Round 2 Dec 9, 2014
@miketaylr
Copy link
Copy Markdown
Member

Hmm, TOTAL: tested 1 platforms, 24/24 tests failed. That seems a little crazy, will need to investigate.

@andrewmcgivery
Copy link
Copy Markdown
Contributor Author

Could it have something to do with the error message at the bottom of the build output?

The command "node_modules/.bin/intern-runner config=tests/functional/intern user="$USER" pw="$PW"" exited with 1.

@miketaylr
Copy link
Copy Markdown
Member

Yeah, the exist code 1 just means it didn't pass (which would be exit 0). It may have just been a hiccup, the other day GitHub was slow and I had some test failures. I'll pull locally and check it out.

@andrewmcgivery
Copy link
Copy Markdown
Contributor Author

Alright well if I screwed something up, please let me know!

@miketaylr
Copy link
Copy Markdown
Member

@andrewmcgivery one thing, can you fix the commit message? Something like:

Issue #464 - Accessibility Fixes Round 2.

That way it will create a link between the commit and the issue. You should be able to do git commit --amend and tweak it, then git push -f.

@miketaylr
Copy link
Copy Markdown
Member

FWIW, all tests pass for me locally. So r+ with the ammended commit message. 🍰

Remainder (Hopefully) of Accessibility fixes.

Issue #464 - Accessibility Fixes Round 2.
@andrewmcgivery
Copy link
Copy Markdown
Contributor Author

Better?

@miketaylr
Copy link
Copy Markdown
Member

Looks good. 👍

Also, I just remembered why the build is failing: the $USER, $PW, $ISSUES_REPO_URI, and $BOT_OAUTH_TOKEN environment variables (which contain a bot's github credentials) aren't exposed to pull requests from forks (more info here) . Kinda crappy, but not the end of the world.

miketaylr pushed a commit that referenced this pull request Dec 11, 2014
Fixes #464: Accessibility Fixes Round 2
@miketaylr miketaylr merged commit 4e31ec8 into webcompat:master Dec 11, 2014
@miketaylr
Copy link
Copy Markdown
Member

Will probably cut a release and deploy to production tomorrow at some point. Thx again for the PR @andrewmcgivery! 🙇

@andrewmcgivery
Copy link
Copy Markdown
Contributor Author

Just to confirm, did I do anything wrong?

And awesome! Can't wait to see these live! :) Thanks!

@miketaylr
Copy link
Copy Markdown
Member

Nope, nothing wrong. ^_^ You can see all tests passing after I merged in your patches: https://travis-ci.org/webcompat/webcompat.com/builds/43674983

@andrewmcgivery
Copy link
Copy Markdown
Contributor Author

Good to know!

@magsout
Copy link
Copy Markdown
Member

magsout commented Dec 11, 2014

👍 again , good job @andrewmcgivery

@andrewmcgivery
Copy link
Copy Markdown
Contributor Author

Thanks! :)

@miketaylr
Copy link
Copy Markdown
Member

Just deployed to production.

@andrewmcgivery
Copy link
Copy Markdown
Contributor Author

Awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Accessibility Fixes Round 2

3 participants