Skip to content

Conversation

@blueyed
Copy link
Contributor

@blueyed blueyed commented Aug 12, 2019

@tyru
Copy link
Member

tyru commented Aug 12, 2019

The important one is the "pr" check (which tests the merge with master).
The other checks the branch as-is (and there it might pass, although it fails for the merge).
Therefore usually testing "pr" for PRs is fine - and happens when creating the PR from a fork (and not the main repo). When using the main repo for PRs also, you usually configure Travis to only build the master branch, and release tags.

hmm, I think so too maybe...
but "push" is really unnecessary...?

how do you think? > @mattn @haya14busa @thinca @ujihisa

@tyru
Copy link
Member

tyru commented Aug 12, 2019

I'm thinking why travis performing "push" by default, if it is totally unnecessary 🤔

@tyru
Copy link
Member

tyru commented Aug 12, 2019

I totally missed when the builds are created.
"push" is created after git push.
"pr" is created after pull request (as you said too).

"push" is useful to build all branches after git push automatically.
but vimlparser is developing in pull request based.
so I think only "pr" is enough.

I approve this in above reason.
after waiting some days, I will merge this.

@blueyed
Copy link
Contributor Author

blueyed commented Aug 12, 2019

I'm thinking why travis performing "push" by default, if it is totally unnecessary

Not really.. it should be done for "master" in most cases.. ;)
And people might use extra branches like "develop", "features" etc that should be build when things get merged into.

The thing here is that you are doing your PRs from branches of the main repo (and not your/a fork).

@ujihisa
Copy link
Member

ujihisa commented Feb 9, 2020

after waiting some days, I will merge this.

181 days have passed. Let's merge this!

@ujihisa ujihisa merged commit 1d130bc into vim-jp:master Feb 9, 2020
@blueyed blueyed deleted the ci-branches branch February 10, 2020 00:00
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.

3 participants