Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

Conversation

@aliak00
Copy link
Contributor

@aliak00 aliak00 commented Jan 18, 2018

On osx I was getting a “No such file” exception for dante.txt
because CWD depended on where I executed rdmd benchmark.d from.
And the object files were generated in a directory also depending
on that.

This fixes obj dir to same level as bin dir within the benchmarks
dir, and allows the runCmd to take a workDir parameter for running
the benchmark executables

On osx I was getting a “No such file” exception for Dante.txt
because CWD depended on where I executed `rdmd benchmark.d` from.
And the object files were generated in a directory also depending
 on that.

This fixes obj dir to same level as bin dir within the benchmarks
dir, and allows the runCmd to take a workDir parameter for running
the benchmark executables
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @aliak00! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

@andralex
Copy link
Member

cc @wilzbach @MartinNowak

{
sw.reset;
auto output = runCmd(cmd, cfg.verbose);
auto output = runCmd(cmd, cfg.verbose, cwd);
Copy link
Contributor

@wilzbach wilzbach Jan 19, 2018

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Argh - I didn't see that cwd is already set to exactly this.

Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

@wilzbach
Copy link
Contributor

CircleCi failure is unrelated - see #2048 for details.

@aliak00
Copy link
Contributor Author

aliak00 commented Jan 19, 2018

Sweet! Thanks 😄

@dlang-bot dlang-bot merged commit b068738 into dlang:master Jan 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants