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

Rerun GitHub Actions on push events to master; update dev documentation to mention a weird thing with JS tests #468

Merged
merged 3 commits into from
Dec 16, 2020

Conversation

fedarko
Copy link
Collaborator

@fedarko fedarko commented Dec 16, 2020

Should fix #467 (I think ... I don't really have a way of testing this).

Also updates CONTRIBUTING.md based on discussion with @gibsramen (we figured out the reason #464 was failing, and the errors being emitted from that reason were pretty obscure -- hopefully this helps clarify things for future devs).

@kwcantrell
Copy link
Collaborator

I did not include the push event because github actions in order to save computation time on the github servers. But it makes sense that it is needed in order to update the status badge. Plus, it will also rerun the on the merged code and check for any weird errors that might pop up.

Comment on lines +38 to +41
If you add HTML elements to EMPress' HTML code that the JavaScript code relies
on, you will also need to add these elements to the `tests/index.html` file.
Ideally this would not be necessary, but for now it is. (Failing to do this can
cause obscure error messages from the JS tests.)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think we should also include a note about issue 435 that causes the legend test to fail if you load tests/index.html into a firefox browser?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That makes sense. Just added a note.

@kwcantrell
Copy link
Collaborator

Thanks @fedarko!

@kwcantrell kwcantrell merged commit a1acaff into biocore:master Dec 16, 2020
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.

It looks like GH actions jobs aren't getting triggered when we merge stuff into master
3 participants