Navigation Menu

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

Flake8 OmeroFS #3118

Merged
merged 21 commits into from Oct 27, 2014
Merged

Flake8 OmeroFS #3118

merged 21 commits into from Oct 27, 2014

Conversation

ximenesuk
Copy link
Contributor

In order to make proposed changes to DropBox it makes sense to have OmeroFS flake8 compliant first. This PR does that. It also moves the fs files to a src directory and modifies the build to handle that.

Tests under OmeroFS should pass and DropBox on three representative platforms should work as normal.

--no-rebase

@sbesson
Copy link
Member

sbesson commented Oct 16, 2014

Is it worth adding flake8 components/tools/OmeroFs/src to the Travis build-python pre-build step?

@sbesson
Copy link
Member

sbesson commented Oct 17, 2014

For some (still mysterious) reason, this PR seems to be at the origin of the failing http://ci.openmicroscopy.org/job/OMERO-5.1-merge-build/ICE=3.4,jdk=7_LATEST,label=ome-c6100-3/ (no omero_version.py) - cf Travis logs. Excluding for today.

@ximenesuk
Copy link
Contributor Author

@sbesson on the first question, once it is fit to merge, yes that was my hope. On the failing build, I saw this once when I tried to run a build locally. I then tried to track it down and couldn't reproduce so I put it down top a local problem. Clearly it isn't! I'll take a look today but any pointers welcome! I assume it has to be a problem with the first two commits. /cc @joshmoore

@ximenesuk
Copy link
Contributor Author

Hmm, it looks like I have killed the copying of omero_version.py to dist/lib/python/ but I'm not sure how, presumably in ximenesuk@73d7728. There is an omero_version.py created in each of the Python tools directories so I'm not clear which one is copied up. I'll investigate!

Previously this file was copied as part of the OmeroFS source
files and was a side-effect of the original flat directory
structure there.
@ximenesuk
Copy link
Contributor Author

This final commit fixes the build so that omero_version.py is copied to the libs directly from OmeroPy. This seems the simplest short term solution with least impact.

@sbesson
Copy link
Member

sbesson commented Oct 17, 2014

Travis log files are encouraging. Reincluding for Monday.

@sbesson sbesson added develop and removed exclude labels Oct 17, 2014
@sbesson
Copy link
Member

sbesson commented Oct 20, 2014

Changeset looks good, the server is properly building and the unit tests are still passing. Proposed next steps for this PR:

@ximenesuk
Copy link
Contributor Author

@sbesson First two points addressed so that you can look at this before I get back. If the PR is good then I'll let you activate the integration jobs.

@sbesson
Copy link
Member

sbesson commented Oct 21, 2014

Thanks @ximenesuk. Will test this locally and reactivate the integration tests.

@sbesson
Copy link
Member

sbesson commented Oct 22, 2014

With sbesson@a79d311 on top of this branch, I am able to run locally

$ ./build.py -f components/tools/OmeroFS/build.xml integration

Want to cherry-pick/duplicate it on this PR before I extend the integration jobs?

@ximenesuk
Copy link
Contributor Author

Done. Thanks for spotting this @sbesson

@sbesson
Copy link
Member

sbesson commented Oct 27, 2014

Dropbox tests now added to the daily integration job. See https://ci.openmicroscopy.org/view/Failing/job/OMERO-5.1-merge-integration-python/280/testReport/test.integration.test_dbclient/TestDropBoxClient/ for the current state. Thanks @ximenesuk. Merging.

sbesson added a commit that referenced this pull request Oct 27, 2014
@sbesson sbesson merged commit 1c19755 into ome:develop Oct 27, 2014
@sbesson sbesson added this to the 5.1.0-m2 milestone Nov 26, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants