Assorted code cleanup #818

Merged
merged 1 commit into from Dec 16, 2016

Projects

None yet

2 participants

@XhmikosR
Member

I tested this, seems to work fine, but please have a close look in case I missed something.

@XhmikosR XhmikosR requested a review from jmervine Dec 12, 2016
@jmervine jmervine requested a deployment to bootstrapcdn-dev-pr-818 Dec 12, 2016 Pending
@jmervine jmervine requested a deployment to bootstrapcdn-dev-pr-818 Dec 12, 2016 Pending
@@ -16,7 +16,7 @@ function sriDigest(file, fromString) {
}
function selectedTheme(config, selected) {
- if (typeof selected === 'undefined' || selected === 'undefined') {
@jmervine
jmervine Dec 12, 2016 Member

I'm pretty sure there was a case where I was returning 'undefined' as a string for selected. Might be long gone now though.

@XhmikosR
XhmikosR Dec 12, 2016 Member

I can't find any relevant code anywhere so it was probably in the past.

@jmervine
jmervine Dec 12, 2016 Member

Okay, then this is probably a safe and a good find at that. =P

@XhmikosR
XhmikosR Dec 12, 2016 Member

I think I found the relevant part https://github.com/MaxCDN/bootstrap-cdn/blob/develop/views/_partials/header.pug#L1

This must have changed in pug and they don't pass an empty string IIRC. Is there any way you could check #818 console log for any errors while browsing the site and switching themes?

@jmervine
jmervine Dec 13, 2016 Member

Yeah, I'll need to do that later this afternoon though.

tests/functional_test.js
@@ -222,8 +214,7 @@ describe('functional', function () {
return;
}
- var domain = helpers.domainCheck('https://maxcdn.bootstrapcdn.com/');
- var uri = domain + root + '/' + name;
+ var uri = 'https://maxcdn.bootstrapcdn.com/' + root + '/' + name;
@jmervine
jmervine Dec 12, 2016 Member

This wasn't hardcoded, because I wanted to be able to test S3 separately when run locally, to debug inconsistencies.

@XhmikosR
XhmikosR Dec 12, 2016 Member

But do you care to keep testing this?

@jmervine
jmervine Dec 12, 2016 Member

I'm on the fence, but I think I want to keep it. This is how I validate uploads to S3 before purging cache.

@jmervine
jmervine Dec 12, 2016 Member

If/when we move off S3, I'd want to nuke this for sure.

@XhmikosR
XhmikosR Dec 12, 2016 Member

All right, I'll see what I can keep from these patches, otherwise I'll close the PR.

@XhmikosR XhmikosR Minor cleanup.
49fc34d
@jmervine jmervine temporarily deployed to bootstrapcdn-dev-pr-818 Dec 12, 2016 Inactive
@XhmikosR
Member

@jmervine; please check one last time.

@jmervine jmervine merged commit b75b688 into develop Dec 16, 2016

5 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
security/snyk No new vulnerabilities
Details
@jmervine jmervine deleted the xmr-code-cleanup branch Dec 16, 2016
@jmervine jmervine added a commit that referenced this pull request Dec 16, 2016
@jdorfman @jmervine jdorfman + jmervine StackPath Branding (#822)
* Switch to a 256x256 image for social media.

512x512 was just too big.

* Update README.md

* Replace footer image and link to StackPath.

* Compress stack-path-icon.svg.

* Inline StackPath logo.

* Use only the used Font Awesome icons. (#806)

* Use only the used Font Awesome icons.

This saves ~80KB from the total page size.

* Minor style tweak for `.fa-flag`.

* Minor cleanup. (#818)
cabbbf5
@jmervine jmervine added a commit that referenced this pull request Dec 16, 2016
@jmervine jmervine Production Release (#823)
* Switch to a 256x256 image for social media.

512x512 was just too big.

* Update README.md

* Replace footer image and link to StackPath.

* Compress stack-path-icon.svg.

* Inline StackPath logo.

* Use only the used Font Awesome icons. (#806)

* Use only the used Font Awesome icons.

This saves ~80KB from the total page size.

* Minor style tweak for `.fa-flag`.

* Minor cleanup. (#818)
c54c87c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment