PyconUK / pyconuk.github.io

OLD REPOS - DO NOT USE. PyCon UK website
http://www.pyconuk.org/
3 stars 9 forks source link

Travis lint for wrongly spelled PyConUK. Partly addresses Issue 36 #38

Closed snim2 closed 9 years ago

snim2 commented 9 years ago

This PR should fail travis, so please don't merge yet

snim2 commented 9 years ago

This looks better to me. Any opinons?

ghickman commented 9 years ago

@snim2 – This looks good to me but one thing I notice is it's not displaying where the mispellings were found. I think that will be useful output in the output of the failure case.

inglesp commented 9 years ago

This looks like a good approach -- but I thought we were spelling it "PyCon UK", with a space.

snim2 commented 9 years ago

Really?

inglesp commented 9 years ago

Yes, according to https://github.com/PyconUK/pyconuk.github.io/commit/3475b4a19872d51a1c04ab79df439c7262e8eeea.

snim2 commented 9 years ago

Oh hell, now it passes again :( Do you want me to squash some of these commits and force-push?

ntoll commented 9 years ago

PyConUK or PyconUK or PyConUk or PyCon UK or Py ConUK or... dammit... ;-) Who cares so long as we're consistent. :-D bikeshed

snim2 commented 9 years ago

So, the last commit gives this result on my machine (Ubuntu 15.04):

$ bash name-lint.sh 
index.html: <img src="static/img/header-home.jpg" alt="Best Ever Pycon UK"> index.html: <h2>Pycon UK Returns!</h2>
Please spell the conference name PyCon UK.

and silently exits with success on Travis. Any ideas?

zeth commented 9 years ago

PyCon UK has always been the name: https://web.archive.org/web/20070427051026/http://www.pyconuk.org/

We picked it to match PyCon US and all the rest :)

On 23 April 2015 at 12:31, Nicholas Tollervey notifications@github.com wrote:

PyConUK or PyconUK or PyConUk or PyCon UK or Py ConUK or... dammit... ;-) Who cares so long as we're consistent. :-D [image: bikeshed] https://cloud.githubusercontent.com/assets/37602/7296129/a809d292-e9b4-11e4-8051-8c12d58d482e.jpg

— Reply to this email directly or view it on GitHub https://github.com/PyconUK/pyconuk.github.io/pull/38#issuecomment-95552858 .

ghickman commented 9 years ago

OK, bikes successfully shed. PyCon UK it is.

@snim2 – I've tested this locally and I'm getting the same result as Travis (no output), however I can find 83 occurrences of Pycon UK in the source. I've fiddled with the command and it looks like specifying the filetype globs is causing the issue.

If you change the commands to egrep "<wrong name>" -R . then I think we're onto a winner!

snim2 commented 9 years ago

@ghickman hmmm, tried the -R in two places and Travis wasn't keen on either.

ghickman commented 9 years ago

@snim2 – I've had a play with this locally and come up with a more succinct version wrapping all the variations up into a single command avoiding Bashisms for any non-bash users.

#!/bin/sh

grep -e "Pycon UK" -e "pycon UK" -e "pyconUK" -e "PyConUK" \
    --recursive . \
    --include "*.html" --include "*.rst"
if [ $? -eq 0 ]; then
    echo "Please spell the conference name PyCon UK."
    exit 1
fi

Hope I'm not stepping on your toes with this!

doismellburning commented 9 years ago

Something's very odd here:

The build of 75a6066 passes - https://travis-ci.org/PyconUK/pyconuk.github.io/builds/59835549

However, index.html at that commit contains the string Pycon UK: https://github.com/snim2/pyconuk.github.io/blob/75a60662cbef295118b0f26e729a7f35958fd4bc/index.html#L10

Running bash name-lint.sh locally, I get an (expected) failure:

$ git show -s
commit 75a60662cbef295118b0f26e729a7f35958fd4bc
Author: snim2 <mount.sarah@gmail.com>
Date:   Fri Apr 24 07:29:18 2015 +0100

    Added PyConUK to list. Use recursive grep without dereference.
$ bash name-lint.sh 
index.html:                <img src="static/img/header-home.jpg" alt="Best Ever Pycon UK">
index.html:                    <h2>Pycon UK Returns!</h2>
Please spell the conference name PyCon UK.

Why on earth is it passing on Travis? :s

snim2 commented 9 years ago

@doismellburning this is why we haven't merged this PR. The lint works just fine on my machine too, so it should really fail correctly on Travis.

I notice that Travis is running Ubuntu 12.04 (I think) and I am on 15.04. I was going to run the lint on a Ubuntu 12.04 Docker container, but I haven't had the chance yet.

doismellburning commented 9 years ago

I'm sorry, I keep a lot of old tabs open, and didn't refresh the page :s

snim2 commented 9 years ago

@doismellburning if you have any other ideas please do shout. I'll have a go at some of @ghickman's ideas when I've cleared my current admin backlog :)

snim2 commented 9 years ago

Looks like we have a winner...

ghickman commented 9 years ago

@snim2 – This looks great, thanks for putting the time in!

d0ugal commented 9 years ago

Probably should have fixed the current offending cases with this :smile:

ghickman commented 9 years ago

@d0ugal – already on it using this handy new tool ;)

ghickman commented 9 years ago

Fixed names in 6a2c9bc

doismellburning commented 9 years ago

@ghickman Please don't do that :( Keep master green!

d0ugal commented 9 years ago

That was my thinking - new checks should come with the fixes needed ideally.

ghickman commented 9 years ago

I'm not personally a fan of rolling new tooling and fixes into one PR but if the consensus is to do that here then happy to do it that way =)

ghickman commented 9 years ago

I've just seen the readme says PRs won't get merged with failing tests so please consider me suitably chastised for my itchy merge finger!

d0ugal commented 9 years ago

:smile: I don't think it matters much, it is just confusing if somebody else is working on changes and sees failures between the tooling and fix PRs.