search

NSLS-II / nslsii

NSLS-II related devices
BSD 3-Clause "New" or "Revised" License
10 stars 22 forks source link

Adding .yml file and enc file for Travis integration and doc creation #43

Closed awalter-bnl closed 5 years ago

awalter-bnl commented 5 years ago

This PR is designed to add a Travis.yml file and a github_deploy_key file to this repo in order to setup the Travis CI.

awalter-bnl commented 5 years ago

Thanks @danielballan and @mrakitin for the quick review of this, much appreciated.

danielballan commented 5 years ago

Wait, it seems like the right solution is not to remove the line installing requirements-dev.txt but to add a requirements-dev.txt file to install. The build is failing because several dev-related packages are not installed.

awalter-bnl commented 5 years ago

I was getting there @mrakitin and @danielballan but thanks for the heads up. I independently started @danielballan's solution after seeing the failure.

danielballan commented 5 years ago

Why 80? 79 is the standard.

awalter-bnl commented 5 years ago

@danielballan I was just seeing if that was the issue, it turns out that wasn't the issue, it was flake8 in general

danielballan commented 5 years ago

Ah, gotcha.

awalter-bnl commented 5 years ago

So I asked the question on slack but will also add it here. The current failure is that the existing code in the repo does not meet flake8 (i.e. pep8) standards and is also failing coverage. Should I go through and fix all of these issues individually? (there is quite a few, I think most files in the repo are failing).

danielballan commented 5 years ago

I vote for ignoring coverage but fixing flake8. It's a young project, might as well start off on the right foot.

As a general comment, I am against bombing old projects with style changes because it obscures the git credit history and risks introducing bugs. So, for example, pandas and IPython occasionally get hit with PRs that suggest ~1000s of lines of style "fixes" and I think they are right to decline them. But I think new projects should start with flake8-compliance and young-ish projects like bluesky that mostly comply can be fixed to literally comply with little downside.

awalter-bnl commented 5 years ago

Appears now that everything is passing flake8, it is just the coverage that is failing.

danielballan commented 5 years ago

Because there are no tests to run and no docs to build, those commands are failing in Travis. I think we should leave all the requirements in place for our future use but comment [out] the lines that attempt to run the tests and build the docs until we have tests to run and docs to builds. Sensible?

awalter-bnl commented 5 years ago

PR #41 has a test in it which is why I added CI to this repo in the first place.

I am not against commenting out the build docs line however