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

Implement single-file packages. Fixes #103. #851

Merged
merged 3 commits into from
May 25, 2016

Conversation

s-ludwig
Copy link
Member

@s-ludwig s-ludwig commented May 24, 2016

See #103.

@s-ludwig
Copy link
Member Author

Inferring the package name requires some additional changes.

#!/usr/bin/env dub
/+ SDL
name "hello_world"
+/
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to have a tested example with dependencies?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. While normal dependencies work fine, while extending the test case, I noticed that path based dependencies are broken. Fixing this cleanly requires some more thought.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 61.092% when pulling a15e2ca on issue103_single_file_packages into 5c4ddfb on master.

@s-ludwig
Copy link
Member Author

Okay, should be ready now. I propose to defer inferring the package name, because that requires some package parser API changes that may or may not be worth it.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 61.092% when pulling cb77aad on issue103_single_file_packages into 5c4ddfb on master.

@PetarKirov
Copy link
Member

Overall LGTM. BTW is dub.json supported as a comment?

@PetarKirov
Copy link
Member

The pretty informative comment on loadSingleFilePackage answered my question. Though a test case would be nice.

@s-ludwig
Copy link
Member Author

Thanks for looking through it! I'll add a dub.json test and rebase + pull.

string file_content = readText(path.toNativeString());
auto idx = file_content.indexOf("/+");
enforce(idx >= 0, "Missing /+ ... +/ recipe comment.");
file_content = file_content[idx+2 .. $].strip();
Copy link
Member

Choose a reason for hiding this comment

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

I think calling strip here is unnecessary, as the call below should cover its purpose.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, that was left-over from an earlier code version.

@s-ludwig s-ludwig force-pushed the issue103_single_file_packages branch from 144299c to 8131dc5 Compare May 25, 2016 08:05
Can be invoked by either running with the --single switch, or by omitting the command and instead specifying a file with a .d extension.
@s-ludwig s-ludwig force-pushed the issue103_single_file_packages branch from 8131dc5 to 810257e Compare May 25, 2016 08:07
@PetarKirov
Copy link
Member

PetarKirov commented May 25, 2016

Thanks for the changes (especially for removing the root_path from loadSingleFilePackage). Now everything LGTM :P

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 61.106% when pulling 372bdad on issue103_single_file_packages into afe62ca on master.

@s-ludwig
Copy link
Member Author

Travis is green again. Merging.

@s-ludwig s-ludwig merged commit d642e93 into master May 25, 2016
@s-ludwig s-ludwig deleted the issue103_single_file_packages branch May 25, 2016 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants