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

Remove deprecated portable paths #2719

Merged
merged 13 commits into from Dec 22, 2016

Conversation

Projects
None yet
2 participants
@rhwood
Contributor

rhwood commented Dec 18, 2016

Addresses #2710 by removing the file: and resource: portable paths from JMRI.

I believe that existing XML files or scripts that still refer to either of these portable paths will throw either a FileNotFoundException or NullPointerException (depending on how they request the actual file) and, unless they silently swallow those errors will either log the file as not findable or display an error. Note that the portable path will simply be passed through as part of the file name in these cases.

A review of the history of FileUtil suggests that only panel files written by JMRI version 2.8 or older will have either of these portable paths in them.

Marked WIP due to uncertainty about how to handle XML schema that allows the removed portable path resource: (this schema also allows file:, but since that also covers the file URL scheme, I am only concerned about resource:).

See #2710 for discussion about decisions taken in this PR.

rhwood added some commits Dec 19, 2016

Catch files generated since 2.8 with invalid portable paths
Files that fail validation are not loaded.
Files generated before 2.8 use a schema that allows arbitrary text
instead of a urlType for icon paths.
First crack at getting useful error handling content
Issues:
- error list is incomplete (especially class not found errors)
- error dialog needs help
Failed on configure xml validation
Failed on configure xml validation due to missing
UserPreferencesManager on CI servers (unable to recreate personally).

@rhwood rhwood added this to the 4.7.1 milestone Dec 19, 2016

@rhwood rhwood requested review from bobjacobsen, pabender and KenC57 Dec 19, 2016

@rhwood

This comment has been minimized.

Show comment
Hide comment
@rhwood

rhwood Dec 19, 2016

Contributor

@bobjacobsen, @KenC57, and @pabender Can you please look over this and see if there are any significant outstanding issues?

I know I am missing some missing classes in the help content, but am not confident I can identify all the missing classes and their replacements. I also don't want to hold this on developing a tool or tools to help fix these issues (I have identified unrelated issues while working on this, the fixes for which would conflict with changes in this PR, so I'd like to merge it sooner rather than later so the other (pending) PRs can also be submitted).

Contributor

rhwood commented Dec 19, 2016

@bobjacobsen, @KenC57, and @pabender Can you please look over this and see if there are any significant outstanding issues?

I know I am missing some missing classes in the help content, but am not confident I can identify all the missing classes and their replacements. I also don't want to hold this on developing a tool or tools to help fix these issues (I have identified unrelated issues while working on this, the fixes for which would conflict with changes in this PR, so I'd like to merge it sooner rather than later so the other (pending) PRs can also be submitted).

@KenC57

This comment has been minimized.

Show comment
Hide comment
@KenC57

KenC57 Dec 20, 2016

Contributor

Generally ok. It seems a few other changes got sucked in, like rps stuff for example, and some editor stuff (rotations). One thing I'm not sure of is if the samples should be translated or used as 'bad' samples to support testing. But not sure how those are supposed to be used here.
-ken c

Contributor

KenC57 commented Dec 20, 2016

Generally ok. It seems a few other changes got sucked in, like rps stuff for example, and some editor stuff (rotations). One thing I'm not sure of is if the samples should be translated or used as 'bad' samples to support testing. But not sure how those are supposed to be used here.
-ken c

@rhwood

This comment has been minimized.

Show comment
Hide comment
@rhwood

rhwood Dec 20, 2016

Contributor

The rps tests fail without these changes--the rps test data was invalid (it contained xml newer than the schema it validated against) and to find that error I had to add the user Config manager to the tests. There's some other cleanup in those tests as well as a result. I've added a sample bad file that is only bad because of invalid paths to the CI tests--most of the other tests files in the CI test suite can used for testing as well.

Contributor

rhwood commented Dec 20, 2016

The rps tests fail without these changes--the rps test data was invalid (it contained xml newer than the schema it validated against) and to find that error I had to add the user Config manager to the tests. There's some other cleanup in those tests as well as a result. I've added a sample bad file that is only bad because of invalid paths to the CI tests--most of the other tests files in the CI test suite can used for testing as well.

@rhwood rhwood removed the WIP label Dec 20, 2016

@rhwood

This comment has been minimized.

Show comment
Hide comment
@rhwood

rhwood Dec 20, 2016

Contributor

The noted rotation changes (in NamedIcon) are a side effect of applying the default formatting instructions to that file after changing JavaDocs that need to be updated to remove a reference to the resource: portable path.

Contributor

rhwood commented Dec 20, 2016

The noted rotation changes (in NamedIcon) are a side effect of applying the default formatting instructions to that file after changing JavaDocs that need to be updated to remove a reference to the resource: portable path.

@KenC57

KenC57 approved these changes Dec 20, 2016

@rhwood rhwood merged commit 40fd535 into JMRI:master Dec 22, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rhwood rhwood deleted the rhwood:deprecated-portable-paths branch Dec 22, 2016

rhwood added a commit to JMRI/website that referenced this pull request Dec 22, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment