Skip to content

Add lint tests for topics#4

Merged
cheshire137 merged 22 commits into
masterfrom
add-testing
Oct 4, 2017
Merged

Add lint tests for topics#4
cheshire137 merged 22 commits into
masterfrom
add-testing

Conversation

@cheshire137
Copy link
Copy Markdown
Member

This adds some basic lint tests to ensure each topic has the data we need. Fixes #3.

We could use more tests that will check image file sizes and dimensions, but that may require some extra work to ensure ImageMagick is set up on the CI server. I'd like to wait till we're hooked up with Travis CI before writing such tests.

@cheshire137 cheshire137 self-assigned this Sep 28, 2017
Comment thread .travis.yml Outdated
install:
- bundle install
notifications:
email: false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rather than globally disabling this I think leaving it to the default for users to be able to receive email notifications bast on their own preferences would be better.

Comment thread README.md Outdated

```bash
bundle install
rake
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

bundle exec rake?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually: would be nice if this was script/cibuild like https://github.com/github/scripts-to-rule-them-all

Comment thread .travis.yml Outdated
email: false
script: rake
cache:
bundler: true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pedantry: I'd order this file sudo, cache, empty line, install, script, empty line, notifications (if you keep it). Totally feel free to ignore me, though.

Comment thread .travis.yml Outdated
- bundle install
notifications:
email: false
script: rake
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would be nice to make this script/cibuild like https://github.com/github/scripts-to-rule-them-all

Comment thread .travis.yml Outdated
@@ -0,0 +1,8 @@
sudo: false
install:
- bundle install
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would be nice to make this script/bootstrap or just part of script/cibuild below like https://github.com/github/scripts-to-rule-them-all

Comment thread README.md Outdated

```bash
bundle install
rake
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually: would be nice if this was script/cibuild like https://github.com/github/scripts-to-rule-them-all

@MikeMcQuaid
Copy link
Copy Markdown
Contributor

@cheshire137 Thoughts on adding (in this PR or another) Rubocop checks to keep any modifications to the tests in a consistent style?

@MikeMcQuaid MikeMcQuaid mentioned this pull request Oct 3, 2017
@cheshire137 cheshire137 merged commit 06c703c into master Oct 4, 2017
MikeMcQuaid pushed a commit that referenced this pull request Jun 28, 2021
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.

5 participants