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

Add Travis CI config #28

Merged
merged 4 commits into from
Sep 5, 2017
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
sudo: false

os:
- osx
- linux
Copy link
Member

Choose a reason for hiding this comment

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

Just building on Linux should be enough. We don't have anything platform dependent.

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 expect anything to break on other platforms" is not the same as "building on other platforms is a detriment". We get it for no additional cost, so I don't see a reason to remove macOS builds.

Copy link
Member

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 responsible to use "free" resources when you don't really need them.

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't "really need" CI builds as a whole, or GitHub collaboration, or any of the other services we use. We've decided to adopt them because they provide benefit to us and our project. The case of building on other platforms is no different. That most of our code isn't special-cased for certain platforms certainly doesn't mean that it won't have platform-specific problems.

Copy link
Member

Choose a reason for hiding this comment

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

You are really wearing me out will all this debate about things that are really not that important in the whole scheme of things (e.g. life, the universe, and everything).

This is a really small project and I haven't felt a need for CI at all. But if you really feel it would be helpful, we can set up travis to do a quick build check on Linux. That is my offer. End of discussion.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are really wearing me out will all this debate about things that are really not that important in the whole scheme of things (e.g. life, the universe, and everything).

I think the debates over relatively small things are a result of the frequent criticism on subjects that are even less consequential. It's things like this that make someone resistant to productive discussion over points that are legitimately relevant. If you make it seem like you're needlessly criticizing someone else's work (whether or not you think it's with good reason), when you have a disagreement over something meaningful neither side is going to be interested in compromise. This is something I've heard voiced by multiple career managers: if you want to produce the best result both for users and your team, you must let the little debates slide so you can push back when you have a strong opinion on a meaningful topic. In the same way that you may think that continuous integration isn't important, I don't think that spaces after ifs or the type of quote I use is. Digging in on one side will inevitably be met with an equal reaction from the other, even if it isn't immediate.

Copy link
Member

Choose a reason for hiding this comment

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

I admit that I am totally guilty of exactly what I am complaining about many times over. And if I have offended because of this, I am sorry. Part of me would like to say more about the specifics of why so many nit picks and what I really meant (certainly not criticism) since that seems to have rubbed you the wrong way, but it seems hypocritical at this point.

So, please keep up the good work and I will do my best to be more mindful of how I come across in reviews.

Copy link
Member

Choose a reason for hiding this comment

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

to be more mindful of how I come across in reviews.

...and to do a better job making it clear what is important to me and what is no big deal.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you. I will certainly try as best as I can from here forward to be receptive to the larger asks and evaluate whether/how there may be a solution that keeps both of us happy so we don't get into binary debates like this. On the topic of formatting and style, I haven't intentionally gone against the format you've laid out but at the same time I haven't spent much of any effort to adhere to it; I'll take a closer look at my code to make sure that I follow that so we aren't leaving ourselves with unspoken issues of that sort.

As for this PR, I'll take a look tomorrow and get it finished up; I plan to remove the macOS builds and keep it to just Linux until we find a need for other platforms. I believe that the OS differences could become significant for us in the future, but as you said it's a value judgment and right now it seems like the simple solution is enough for the vast majority of cases.


language: node_js
node_js: lts/*

before_install:
- if [ $TRAVIS_OS_NAME == "linux" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

We probably don't need any of this.

We don't use any native modules, so CXX and CC are not needed.

I doubt need X either.

Copy link
Member Author

Choose a reason for hiding this comment

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

These were for running tests, which we aren't doing.

export CXX="g++-4.9" CC="gcc-4.9" DISPLAY=:99.0;
sh -e /etc/init.d/xvfb start;
sleep 3;
fi

install:
- npm install
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be npm install --no-optional.

Copy link
Member Author

Choose a reason for hiding this comment

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

The point of optional dependencies is that they are optional... i.e., you don't need to specify that flag.


script:
- npm run vscode:prepublish