Skip to content

Conversation

w0rp
Copy link
Contributor

@w0rp w0rp commented Apr 18, 2015

Related to a recent discussion, this patch will add a command to dub to list all of the import paths used
for a project, line-by-line.

There might be some details I missed here, so I would appreciate some input. I have next to no experience with the DUB codebase.

@MartinNowak
Copy link
Member

I think it would be nicer to add this as dub describe --import-paths and dub describe --string-import-paths rather than adding lots of new commands.

@MartinNowak
Copy link
Member

A test for this and dub describe would be nice as well, https://coveralls.io/builds/2369240/source?filename=source%2Fdub%2Fcommandline.d#L761.
See 0-init-simple.sh or test-version-opt.sh for examples.

@w0rp
Copy link
Contributor Author

w0rp commented Apr 19, 2015

Okay then. I'll see about adding the command as switches on dub describe instead. I'll see if I can add something for --string-import-paths. I believe it's not so hard for me to grab those either. I saw the message before about decreased code coverage, and I was wondering how to add tests for the project. It's slightly more complicated than my usual process of adding a unittest block.

I might have to come back to this during the week, but I'll get it done eventually.

@MartinNowak
Copy link
Member

I was wondering how to add tests for the project

Just add a shell script, dub init a project, run dub describe --import-paths and grep for the source paths or so, see test-version-opt.sh.

@MartinNowak
Copy link
Member

You found some time to implement it?

…ribe' instead of a new command. Add some scripts for testing the output of each, and a smoke test for 'dub describe'.
@w0rp
Copy link
Contributor Author

w0rp commented Apr 26, 2015

I just updated this again with the options as you requested. I added some test scripts for checking the output of each option, and a smoke test for dub describe. Maybe something could be done to test the JSON output for dub describe.

I modified the script which runs the other test scripts so the die and log functions can be used in them, as well as the CURR_DIR variable.

I'll see if I can figure out why the Travis CI script works on my machine, but fails on Travis CI.

@w0rp
Copy link
Contributor Author

w0rp commented Apr 26, 2015

There we go. I was dumb and committed my absolute paths, so now the scripts generate them in the environment. I also needed to add some dummy files so the right directories will be created, for the paths dub uses automatically.

source/dub/dub.d Outdated
Copy link
Member

Choose a reason for hiding this comment

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

You can return the paths array insyead of making it an output parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can do. I was just following the way the other methods in the class were using output parameters.

@MartinNowak
Copy link
Member

Looks good, thanks.
If you wanted, you could use jq or grep to test the describe output.

@w0rp
Copy link
Contributor Author

w0rp commented Apr 27, 2015

I updated the D source files and the Bash scripts now. I created some very basic tests with jq, but I couldn't seem to get Travis to use it. I did see a whitelist for apt packages on Travis, but jq doesn't appear to be in it. I've pulled that out for now.

@MartinNowak
Copy link
Member

It should be preinstalled (http://docs.travis-ci.com/user/build-environment-updates/2014-12-09/#Additions), but not too important right now.

MartinNowak added a commit that referenced this pull request Apr 29, 2015
Add a command to DUB to list all of the import paths for a project.
@MartinNowak MartinNowak merged commit e61a104 into dlang:master Apr 29, 2015
@Abscissa
Copy link
Contributor

Abscissa commented May 1, 2015

W000t! This is awesome! Thanks a ton for this, @w0rp and @MartinNowak!!

Copy link
Contributor

Choose a reason for hiding this comment

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

an = and?

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