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.

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

index.html: Comment out setup/ link to make installation-tests opt-in #278

Merged
merged 1 commit into from
Jan 27, 2016

Conversation

wking
Copy link
Contributor

@wking wking commented Jan 10, 2016

Sometimes instructors forget to customize their CHECKS, but learners
still follow this link and run the stock checks, resulting in a bunch
of confusing errors about missing packages that the learner
doesn't actually need. Instead of expecting
instructors to edit CHECKS or remove the setup/ reference from
index.html (a default that is right for nobody), this commit comments
that setup/ reference out (a default that is right for folks who don't
want installation-checks). Instructors who do want installation
checks now need to do two things (uncomment the setup/ reference and
update CHECKS), but with both steps listed in CUSTOMIZATION they are
more likely to get that right.

Spun off from this thread.

Sometimes instructors forget to customize their CHECKS, but learners
still follow this link and run the stock checks, resulting in a bunch
of confusing errors about missing packages that the learner doesn't
actually need [1,2,3,4].  Instead of expecting instructors to edit
CHECKS or remove the setup/ reference from index.html (a default that
is right for nobody), this commit comments that setup/ reference out
(a default that is right for folks who don't want
installation-checks).  Instructors who *do* want installation checks
now need to do two things (uncomment the setup/ reference and update
CHECKS), but with both steps listed in CUSTOMIZATION they are more
likely to get that right.

[1]: carpentries#136
[2]: carpentries#180
[3]: carpentries#181
[4]: carpentries#258
@iglpdc
Copy link
Contributor

iglpdc commented Jan 10, 2016

I'm +1, but as a temporary fix before we agree in a solution to handle the checking in an automated way.

@wking
Copy link
Contributor Author

wking commented Jan 11, 2016

On Sun, Jan 10, 2016 at 03:59:36PM -0800, Ivan Gonzalez wrote:

I'm +1, but as a temporary fix before we agree in a solution to
handle the checking in an automated way.

Greg suggests that some folks may not want installation testing at all
1, in which case we'd want to keep this around even after something
like wking/swc-setup-installation-test#9 lands.

@iglpdc
Copy link
Contributor

iglpdc commented Jan 11, 2016

Ok, full +1 then... :)

On Mon, Jan 11, 2016 at 1:07 AM, W. Trevor King [email protected]
wrote:

On Sun, Jan 10, 2016 at 03:59:36PM -0800, Ivan Gonzalez wrote:

I'm +1, but as a temporary fix before we agree in a solution to
handle the checking in an automated way.

Greg suggests that some folks may not want installation testing at all
1, in which case we'd want to keep this around even after something
like wking/swc-setup-installation-test#9 lands.


Reply to this email directly or view it on GitHub
#278 (comment)
.

@gvwilson
Copy link
Contributor

gvwilson commented Jan 11, 2016 via email

@gvwilson
Copy link
Contributor

gvwilson commented Jan 11, 2016 via email

@wking
Copy link
Contributor Author

wking commented Jan 11, 2016

On Mon, Jan 11, 2016 at 11:08:20AM -0800, Greg Wilson wrote:

I'm -1 until we've tested this with novice instructors.

Do we have a plan for testing this? With a single workshop-template
branch/repo used by both existing and new instructors, I don't see an
easy way to do A/B testing.

@gvwilson
Copy link
Contributor

gvwilson commented Jan 11, 2016 via email

@rgaiacs
Copy link
Contributor

rgaiacs commented Jan 23, 2016

Sorry for the delay.

@wking Thanks for the pull request. I'm +1 to merge this.

@gvwilson I add a remember to merge this on January 25th if I don't hear anything from you.

rgaiacs added a commit that referenced this pull request Jan 27, 2016
index.html: Comment out setup/ link to make installation-tests opt-in
@rgaiacs rgaiacs merged commit db053ba into carpentries:gh-pages Jan 27, 2016
@wking wking deleted the installation-test-op-in branch January 29, 2016 21:10
wking added a commit to wking/workshop-template that referenced this pull request Mar 11, 2016
…tions

Ivan brought this idea back up in maintainers@ recently [1].  I think
the last time it was raised seriously was [2], which has reasonable
links into earlier discussion.  The main argument against tags was
that it was too hard to find the source for a particular line you
wanted to tweak [3].  This commit restores our old liquid templating
to show/hide sections *without* splitting the sections out into
sub-files (e.g. bc#738 had _includes/setup/linux-editor.html).  If we
keep everything in the index file, we can have tags and instructors
can either adjust the tags or easily find/edit/delete as they see fit.

A few notes on the implementation:

* I've gone with double quotes in the YAML front matter for
  consistency with the other entries, but stuck with the original
  (from swcarpentry/bc) single quotes for the liquid conditionals.

* I've kept "check" and "VM" out of the default tools list to match
  the current display, but we may want to enable everything and write
  a stronger message about removing stuff you don't need to avoid
  repeating the problems we had with check being visible by default
  [4].  Because folks will have to tweak the tools list if they want
  to enable the "check" or "VM" sections, I've added comments at the
  beginning of each section pointing instructors back up at the YAML
  front matter.

* I've moved the "check" section out of the Python section, because
  while the tool doesn't currently test R packages, it does test Git,
  Bash, text editors, etc., and it could certainly be extended to test
  R if someone with R knowledge wanted to chip in (although it's
  harder to *run* the script on Windows without bundling Python) [5].

[1]: http://lists.software-carpentry.org/pipermail/maintainers_lists.software-carpentry.org/2016-March/000179.html
     Subject: Re: [Maintainers] Vote needed for setup instructions
     Date: Thu, 10 Mar 2016 12:05:33 -0500
     Message-ID: <CAJpTZ0DPYKgu2fbi6dZ3XvZVyed0MwmS-jTR-e-Y3uYoZKoctQ@mail.gmail.com>
[2]: swcarpentry/DEPRECATED-bc#738
[3]: swcarpentry/DEPRECATED-bc#729 (comment)
[4]: carpentries#278
[5]: carpentries#136 (comment)
wking added a commit to wking/workshop-template that referenced this pull request Mar 11, 2016
…tions

Ivan brought this idea back up in maintainers@ recently [1].  I think
the last time it was raised seriously was [2], which has reasonable
links into earlier discussion.  The main argument against tags was
that it was too hard to find the source for a particular line you
wanted to tweak [3].  This commit restores our old liquid templating
to show/hide sections *without* splitting the sections out into
sub-files (e.g. bc#738 had _includes/setup/linux-editor.html).  If we
keep everything in the index file, we can have tags and instructors
can either adjust the tags or easily find/edit/delete as they see fit.

A few notes on the implementation:

* I've gone with double quotes in the YAML front matter for
  consistency with the other entries, but stuck with the original
  (from swcarpentry/bc) single quotes for the liquid conditionals.

* I've kept "test" and "VM" out of the default tools list to match the
  current display, but we may want to enable everything and write a
  stronger message about removing stuff you don't need to avoid
  repeating the problems we had with the installation-test link being
  visible by default [4].  Because folks will have to tweak the tools
  list if they want to enable the "test" or "VM" sections, I've added
  comments at the beginning of each section pointing instructors back
  up at the YAML front matter.

* I've moved the "test" section out of the Python section, because
  while the tool doesn't currently test R packages, it does test Git,
  Bash, text editors, etc., and it could certainly be extended to test
  R if someone with R knowledge wanted to chip in (although it's
  harder to *run* the script on Windows without bundling Python) [5].
  The new header separates the test section from the previous section.

[1]: http://lists.software-carpentry.org/pipermail/maintainers_lists.software-carpentry.org/2016-March/000179.html
     Subject: Re: [Maintainers] Vote needed for setup instructions
     Date: Thu, 10 Mar 2016 12:05:33 -0500
     Message-ID: <CAJpTZ0DPYKgu2fbi6dZ3XvZVyed0MwmS-jTR-e-Y3uYoZKoctQ@mail.gmail.com>
[2]: swcarpentry/DEPRECATED-bc#738
[3]: swcarpentry/DEPRECATED-bc#729 (comment)
[4]: carpentries#278
[5]: carpentries#136 (comment)
fmichonneau pushed a commit that referenced this pull request Jun 20, 2018
Run lesson-fixme from within lesson-check
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.

4 participants