Quinn-Interactive / silverstripe-seo

An all-in-one SEO module for SilverStripe 4.1+
BSD 3-Clause "New" or "Revised" License
33 stars 20 forks source link

Fix and add CI Enhancement #24

Closed gordonbanderson closed 6 years ago

gordonbanderson commented 6 years ago

hi,

I ran into an issue whereby the site just hung when editing due to the public URL not being the same as the site URL from a docker container perspective. As such, this provides an optional configuration override to change the URL. Documented in README, which I line break length fixed.

As an extra I've added a Circle CI file, tests were taking around a minute, and are passing for the one combination provided in the config file.

Cheers

Gordon

gordonbanderson commented 6 years ago

See https://travis-ci.org/gordonbanderson/silverstripe-seo-1/jobs/373749675 for Circle CI report

codecov-io commented 6 years ago

Codecov Report

Merging #24 into master will decrease coverage by 0.22%. The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #24      +/-   ##
============================================
- Coverage     33.78%   33.55%   -0.23%     
- Complexity      174      175       +1     
============================================
  Files            16       16              
  Lines           447      450       +3     
============================================
  Hits            151      151              
- Misses          296      299       +3
Impacted Files Coverage Δ Complexity Δ
src/Extensions/PageHealthExtension.php 0% <0%> (ø) 7 <0> (+1) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update da9fff1...59f0364. Read the comment docs.

zanderwar commented 6 years ago

Nice @gordonbanderson thank you.

gordonbanderson commented 6 years ago

The owner of the repo can do the squash merge, that's the way I normally go. Too lazy to git rebase -i ;) I have just discovered a bug though, it fails on new pages as they are not publicly visible. As such hold off on a merge of any sort.

zanderwar commented 6 years ago

Sure that's fine, I can do that.

Didn't know that you could squash with rebase :octocat:

This is how I normally squash:

git log
git reset --soft COMMIT_HASH
git add .
git commit -m "FEATURE: blah"
git push -f

I've refactored a lot of this rendered html stuff in master and was able to retain backwards compatibility without bumping the major version, so you'll likely see this stuff (albeit slightly altered to suit) in v1.1

I contemplated adding draft/versioning support for the analyses etc but yet to think of a decent way to do that.

gordonbanderson commented 6 years ago

^^ That looks way faster than git rebase -i :)

gordonbanderson commented 6 years ago

I cannot immediately see a solution to this problem, try overriding 'nginx' name of container to the same as docker inspect, but no dice. Don't have time to debug now, will revisit later.