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

get tut to work with Travis #593

Merged
merged 10 commits into from
Oct 7, 2016
Merged

get tut to work with Travis #593

merged 10 commits into from
Oct 7, 2016

Conversation

jarrodu
Copy link
Member

@jarrodu jarrodu commented Oct 3, 2016

Fix #591

I went with @SethTisue idea of letting Travis do all the work and not using tut for building the site.

Currently this will take all the files in the repo and pass them to tut. Tut will validate all code blocks that tut recognizes. If there is an error then the Travis build will fail. If tut does succeed in validating all the files then Travis will build the site like before. None of the output files of tut are discarded.

At the moment none of the tutorials use code blocks that tut recognizes. At some point in the future the old tutorials could be modified to allow for the checks. All future tutorials should use the correct type of code blocks.

@SethTisue
Copy link
Member

SethTisue commented Oct 3, 2016

At some point in the future the old tutorials could be modified to allow for the checks

this looks cool — but I'm not sure it makes sense to merge unless/until it includes at least one example where tut is actually doing its job? for two reasons, 1) so we can know it's even working (by inserting an error into the example, running tut, and seeing the failure), 2) as an example for others to follow

@SethTisue
Copy link
Member

also, is there is somewhere in this repo this could/should be documented?

@jarrodu
Copy link
Member Author

jarrodu commented Oct 3, 2016

What tutorial would you like me to modify?

@SethTisue
Copy link
Member

pick one or more sections of the language tour, I guess? judging from the amount of Disqus comments those pages get, at least, they seem to get an especially large amount of traffic

The code blocks with `tut`, `tut:fail`, or `tut:nofail` are checked by tut. The others are not.
This is to demonstrate that Travis will catch tut errors.
@jarrodu
Copy link
Member Author

jarrodu commented Oct 4, 2016

@SethTisue I implemented your suggestions and the PR is ready for review.

@soc
Copy link
Contributor

soc commented Oct 4, 2016

@jarrodwb Is there a way to configure tut to use a different code block (like ```scala?)

@jarrodu
Copy link
Member Author

jarrodu commented Oct 4, 2016

I am not sure. Tut has a gitter channel though.

@@ -1,5 +1,4 @@
# opt-in to Travis new infrastructure
sudo: false
sudo: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to be able to use sudo and similar commands. The main thing is to be able to chmod coursier. This is disallowed on the container version of Travis. The sudo: true puts it all on a virtual machine. At least this is my understanding.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can't even chmod +x things we just downloaded ourselves in our own home directory? that surprises me, I wouldn't expect superuser privileges to be necessary. are you sure?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is our home directory. It is Travis's directory and Travis executes our scripts. At least this is how I think about it.

https://docs.travis-ci.com/user/ci-environment/

The container builds are heavily restricted. You can't even install all apt packages.

https://github.com/travis-ci/apt-package-whitelist

I did not look to deeply into it. If you remove the sudo: true Travis will throw an error when you try to chmod. There maybe another way to get around this error. Feel feel to play around with it.

@@ -39,6 +39,8 @@ It's statically generated from [Markdown](http://en.wikipedia.org/wiki/Markdown)

The markdown syntax being used supports [Maruku](http://maruku.rubyforge.org/maruku.html) extensions, and has automatic syntax highlighting, without the need for any tags.

Additionally [tut](https://github.com/tpolecat/tut) is used during pull requests to valid Scala code blocks. To use this feature you must use the backtick notation as documented by tut. Note that only validation are done. The output files from tut are not used in the building of the tutorial. Either use `tut` or `tut:fail` for your code blocks.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/valid/validate/
s/are done/is done/

@SethTisue
Copy link
Member

@tpolecat does this look reasonable to you?

@SethTisue
Copy link
Member

@jarrodwb have you verified that Travis fails if there's an error in one of the tut sections?

@tpolecat
Copy link

tpolecat commented Oct 5, 2016

Why are you not using the output?

two grammar changes
@jarrodu
Copy link
Member Author

jarrodu commented Oct 5, 2016

@SethTisue Yes. I am sure that Travis fails when tut throws an exception. I tested it out locally and also on this PR. There a few commits where I submitted broken code and then fixed it again. 990085e and 3a052b6

@jarrodu
Copy link
Member Author

jarrodu commented Oct 5, 2016

@tpolecat The main reason is that the current project is pretty old and it would take a lot of changes to use tut to preprocess the tutorials. The current tutorials don't use GFM code blocks and were written without the idea of having the output of code blocks inserted into the site. Also sbt-site and sbt are currently not being used to generate the site. I would have had to completely redo the directory structure to be able to use the more modern approach. In the future hopefully we can go in this direction. If a new tutorial is written and added to the site then it would make sense to use the output of tut. Currently we are only dealing with rather old tutorials that are only being maintained and not expanded.

One other major reason that @SethTisue asked me to do in this way. ;) See #591

@SethTisue
Copy link
Member

@tpolecat our main concern about using the output was that the deployment pipeline would have to change, as discussed at #591. we might still want to go on and do that, but we were hoping to get this much integrated and merged first before worrying too much about a more ambitious option

@tpolecat
Copy link

tpolecat commented Oct 6, 2016

@SethTisue this looks reasonable to me, as long as the final rendering engine can cope with the '''tut markup … I didn't see anything in the PR that rewrites those. There is an old verify-only mode as a feature request that you can feel free to upvote. It might have made this easier for you.

@SethTisue
Copy link
Member

as long as the final rendering engine can cope with the '''tut markup

when I tested this locally it didn't seem to faze Kramdown. @jarrodwb, you have seen the same in your testing? perhaps you could check and see if our bundle exec... setup is still in sync with whatever GitHub Pages is using these days? because if it isn't, we could get a nasty surprise after this was merged.

@jarrodu
Copy link
Member Author

jarrodu commented Oct 6, 2016

Everything works for me locally. I will double check GitHub Pages and see what I can come up with.

@jarrodu
Copy link
Member Author

jarrodu commented Oct 6, 2016

Looks good to me. When we use the github-pages gem we get a local version of the same setup that GitHub uses. If this gem works they way it is advertised then we are good to go. :)

https://github.com/github/pages-gem

@jarrodu
Copy link
Member Author

jarrodu commented Oct 7, 2016

@SethTisue Any chance we can get this merged in?

@SethTisue SethTisue merged commit 8ad03ec into scala:master Oct 7, 2016
@SethTisue
Copy link
Member

🎉 thanks Jarrod!

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