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

fix outdir for nim PR #46

Merged
merged 1 commit into from
Feb 12, 2020
Merged

fix outdir for nim PR #46

merged 1 commit into from
Feb 12, 2020

Conversation

timotheecour
Copy link
Contributor

@timotheecour timotheecour commented Feb 11, 2020

/cc @jangko
seems like this is the only nimble package that fails for this nim PR: nim-lang/Nim#13382
after this fix, nim-lang/Nim#13382 should have its CI pass

let me know if you have any questions

@jangko jangko merged commit dd5d816 into jangko:master Feb 12, 2020
@jangko
Copy link
Owner

jangko commented Feb 12, 2020

it does not pass the CI test, somehow it didn't trigger CI build.

@jangko
Copy link
Owner

jangko commented Feb 12, 2020

the $projectdir in --outdir:'$projectdir' not replaced by actual path on Windows

@timotheecour timotheecour deleted the pr_fix_outdir branch February 12, 2020 02:59
@timotheecour
Copy link
Contributor Author

timotheecour commented Feb 12, 2020

/cc @jangko hmm, that's too bad... I'm seeing a few possible choices to fix this:

  • add a tests/test_json.cfg file containing:
    --outdir:"$projectdir"
    not sure whether this will help, depends on whether the windows fail was due to quoting issues or to $projectdir substitution just not working at all on windows
  • expose getNimbleFile via a magic in nim (I added it for internal use for docgen in fix #13150 nim doc --project now works reliably nim-lang/Nim#13223 but it would actually be useful to solve this, giving a reference path that works however the package is installed (locally via nimble develop or installed via nimble install0
  • change a bit the logic in test_json.nim

thoughts?

EDIT: see #47

@timotheecour timotheecour mentioned this pull request Feb 12, 2020
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.

2 participants