Skip to content

Travis CI: Simplify the testing #887

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jul 3, 2019
Merged

Travis CI: Simplify the testing #887

merged 5 commits into from
Jul 3, 2019

Conversation

cclauss
Copy link
Member

@cclauss cclauss commented Jun 8, 2019 •
edited
Loading 

language: python
install: pip install flake8
script: flake8 . --count --select=E9,F63,F72,F82 --show-source --statistics

@poyea
Copy link
Member

poyea commented Jun 8, 2019

I think the repository only enables LGTM and maybe there's no need to use Travis anyone.

@cclauss
Copy link
Member Author

cclauss commented Jun 8, 2019 •
edited
Loading

The problems are that:

  1. LGTM does not want us running flake8 on their infrstructure and
  2. when we do, it does not work as expected.

This can be seen in #882 which failed the LGTM tests even though it fixed the syntax error. It was only after #882 was merged that the LGTM tests started to pass. We want the test runner to test the code the current pull request but LGTM always tests the master branch. Travis CI (this PR) or CircleCI (#888) will test the current pull request and tell us if the code in that pull request should be accepted or not.

@poyea
Copy link
Member

poyea commented Jun 17, 2019

I'll leave this to the owners. @AnupKumarPanwar

@cclauss
Copy link
Member Author

cclauss commented Jul 1, 2019 •
edited
Loading

@AnupKumarPanwar Can we please get https://travis-ci.org/TheAlgorithms/Python re-enabled? It ran 21 days ago.

@AnupKumarPanwar
Copy link
Member

@cclauss It's already enabled.

@cclauss
Copy link
Member Author

cclauss commented Jul 2, 2019

Great! Thanks.

@AnupKumarPanwar
Copy link
Member

AnupKumarPanwar commented Jul 2, 2019 •
edited
Loading

@cclauss @poyea I don't suggest using multiple CI tools. We can discuss which one to use. Which CI tool do you guys this is most appropriate for the repo?

@cclauss
Copy link
Member Author

cclauss commented Jul 3, 2019

Can we please get this PR reviewed and merged? Now that Travis CI has been enabled all of our new PRs are failing which slows down the contribution and review process. This PR should get them working again.

I have no problem running multiple CI tools (many projects do so) but once this PR has landed, we can disable LGTM if desired. Travis CI is clearly more flexible than LGTM.

@cclauss
Delete .lgtm.yml
5b3d880 
These changes were created to get around the fact that Travis CI was not enabled on this repo.  Now that Travis is enabled, we can remove these modifications.

The problems are that:
1. [LGTM does not want us running flake8 on their infrstructure](https://discuss.lgtm.com/t/can-i-get-lgtm-to-run-flake8-tests/1445/6) and
2. when we do, it [does not work as expected](https://discuss.lgtm.com/t/tests-are-not-automatically-run-when-lgtm-yml-is-modified/1446/4).
@AnupKumarPanwar AnupKumarPanwar merged commit 4fb4cb4 into TheAlgorithms:master Jul 3, 2019
@cclauss cclauss deleted the patch-3 branch July 3, 2019 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnupKumarPanwar AnupKumarPanwar AnupKumarPanwar approved these changes

Assignees

@AnupKumarPanwar AnupKumarPanwar

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cclauss @poyea @AnupKumarPanwar