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 some sort of CI to this repo? #47

Open
dmitshur opened this issue Feb 22, 2016 · 6 comments
Open

Add some sort of CI to this repo? #47

dmitshur opened this issue Feb 22, 2016 · 6 comments

Comments

@dmitshur
Copy link
Member

/cc @dominikh

To be discussed.

@dmitshur
Copy link
Member Author

From https://github.com/dominikh/gopherjs.github.io/commit/57a5c6bb6d6b07687ab2d1ebc38d726486bb25fd#commitcomment-16230320:

I was actually considering adding checking that the playground can be built to the CI of main gopherjs repository. I know @neelance usually checks that before merging to master, and it would help catch breakages like cfcb38d. Checking CI in another repo is less visible when changing gopherjs repo IMO.

@dmitshur
Copy link
Member Author

In fact, I'd like to consider adding a few supported and "blessed" external packages to the CI of main gopherjs repository, for example:

However, ideally I want to consolidate gopherjslib and gopherjs_http.NewFS and the internal http.FileSystem that's used by gopherjs serve command. So gopherjslib and gopherjs_http won't be an external repo after that.

@dominikh
Copy link
Member

Checking CI in another repo is less visible when changing gopherjs repo IMO.

It can also be said that checking the playground only in the main repo's CI is not visible, either. It wouldn't have detected the mktemp issue until GopherJS changed, and once it changed, the CI failure would've been confusing.

Optimally, it would be checked in both. The playground's CI would check that changes to the playground don't break it, and GopherJS's CI would check that changes to GopherJS don't break the playground.

@dmitshur
Copy link
Member Author

It can also be said that checking the playground only in the main repo's CI is not visible, either.
...
Optimally, it would be checked in both.

Yeah, I didn't mean that adding playground-specific CI check to gopherjs repo precludes adding normal CI (and a badge) to this repo. They're independent things.

What you said above makes sense in general, but I have a concern... Usually CI is not used to test that go generate step works. Developers do that. go generate may require custom tools and setup that is not generally available. CI is usually testing that building works (and tests pass). So, it may or may not be a good idea to use CI for testing go generate directives.

@dmitshur
Copy link
Member Author

In other words, I think it may be more optimal to have developers review changes to go generate work, since they'll be the only ones ever running that.

But changes to code can and should be tested for build errors, etc., via CI. This is both easy to add and sustainable to maintain.

@dominikh
Copy link
Member

Yes, you are right.

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

No branches or pull requests

2 participants