Skip to content

Comments

TC_INITSCRIPT support absolute/relative path#904

Closed
basicfu wants to merge 0 commit intotestcontainers:masterfrom
basicfu:master
Closed

TC_INITSCRIPT support absolute/relative path#904
basicfu wants to merge 0 commit intotestcontainers:masterfrom
basicfu:master

Conversation

@basicfu
Copy link

@basicfu basicfu commented Oct 8, 2018

see #850
TC_INITSCRIPT Add two path way

  • begin with root: prefix,relative root path
  • use absolute path

@rnorth
Copy link
Member

rnorth commented Oct 10, 2018

This is a good idea - thanks for raising the PR!

I'm just not quite sure about the root: prefix; 'root' sounds (to me) very different from the relative path that it's using.

How about file: as an alternative name? The current IETF draft for the file scheme seems to tolerate relative path file URIs (like file:somedir/somefile). There's the question of whether it should or should not support full absolute paths (like file:///home/user/somedir/somefile), but I'd say we could live without that.

@basicfu
Copy link
Author

basicfu commented Oct 10, 2018

I agree with file: as an alternative name.
full absolute paths it seems to be useless,Then I delete absolute paths?

@testcontainers testcontainers deleted a comment Oct 10, 2018
@testcontainers testcontainers deleted a comment Oct 12, 2018
@basicfu
Copy link
Author

basicfu commented Dec 21, 2018

Hi @kiview, I have already modified

URL resource;
if (initScriptPath.startsWith(FILE_PATH_PREFIX)) {
//relative project path
String relativePath = System.getProperty("user.dir") + "/" + initScriptPath.substring(FILE_PATH_PREFIX.length());
Copy link
Member

Choose a reason for hiding this comment

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

why are you using user.dir here?
Also, why not just return new URL(initScriptPath)? :)

Copy link
Author

Choose a reason for hiding this comment

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

use user.dir Use the project path

@rnorth
Copy link
Member

rnorth commented Mar 19, 2019

I'll pick this up - will remove user.dir and add docs.

@rnorth
Copy link
Member

rnorth commented Mar 19, 2019

Sorry, I didn't mean to close this PR. I've reopened as #1329 with a couple of tweaks.

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.

4 participants