-
Notifications
You must be signed in to change notification settings - Fork 0
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
Setting up CI #79
Setting up CI #79
Conversation
Wow that's a lot of commits. Maybe squash? |
packages/RichTextEditing-Core.package/RichTextDocument.class/instance/sortRunsByPriority.st
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems alright, pls explain what you did
True. But all of them contain only minor changes, since when changing files on Github directly, every change produces an separate commit. |
In overview:
|
Squashing can be done online, when the pull request is merged, by clicking the arrow next to "Merge pull request" |
@@ -1,4 +1,4 @@ | |||
# RichTextEditing [![Build Status](https://travis-ci.org/hpi-swa-teaching/RichTextEditing.svg?branch=master)](https://travis-ci.org/hpi-swa-teaching/RichTextEditing)[![Coverage Status](https://coveralls.io/repos/github/hpi-swa-teaching/RichTextEditing/badge.svg?branch=master)](https://coveralls.io/github/hpi-swa-teaching/RichTextEditing?branch=master) | |||
# RichTextEditing ![CI](https://github.com/hpi-swa-teaching/RichTextEditing/workflows/CI/badge.svg)[![Build Status](https://travis-ci.org/hpi-swa-teaching/RichTextEditing.svg?branch=dev)](https://travis-ci.org/hpi-swa-teaching/RichTextEditing)[![Coverage Status](https://coveralls.io/repos/github/hpi-swa-teaching/RichTextEditing/badge.svg?branch=master)](https://coveralls.io/github/hpi-swa-teaching/RichTextEditing?branch=master) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don’t need travis anymore, so the badge can surely be removed
- Squeak-5.2 | ||
- Squeak-5.1 | ||
- Squeak64-trunk | ||
- Squeak64-5.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we just remove travis at this point, as we have GitHub actions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite sure about this, since without travis, coverage is no longer reported.
Travis is actually succeeding, the badge only shows it as failing because it is already targeting the dev branch with the old travis configuration. Does not hurt to keep it, I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, github action has no coverage report?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like it. The smalltalkci readme does only mention coverage for Travis or AppVeyor and the test-ci branch only appearend on coveralls after I reenabled travis.
It’s better to do a rebase when pulling the changes from dev branch to avoid merge commits |
.github/workflows/main.yml
Outdated
# events but only for the master branch | ||
on: | ||
push: | ||
branches: [ dev ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI should be running on each push on each branch. This allows for test driven development and warns you if you pushed something that breaks a test etc. If you run the CI only on pull requests, you might already have done a lot and it's unclear on which commit it exactly got wrong.
Since the tests seem to be not run there
With a lot of trial and error, CI now finally seems to work.
I haven't noticed any difference between github actions and travis though; both seem to now work fine and produce the same results with the main difference being that travis reports coverage to coveralls.io while github actions does not.
When merged, the CI configuration and the README.md badges should now target the dev branch.
Note that I also had to change one line of source code to make all current tests succeed.
Closes #74.