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

Add --path flag #53

Merged
merged 8 commits into from
Nov 21, 2019
Merged

Conversation

ordepdev
Copy link
Contributor

This change allows us to provide a directory for creating and loading both corral.json and dep-lock.json files. It affects most commands as they mostly need to load the file before doing their job. So, from now on, we can pass the --path "path/to/the/bundle/dir/" argument.

corral[master] % mkdir tmp
corral[master] % corral init --path tmp

init: from dir tmp
INFO: Created bundle in /Users/ordepdev/Work/corral/tmp
FINE: Going to write /Users/ordepdev/Work/corral/tmp/corral.json
INFO: Writing /Users/ordepdev/Work/corral/tmp/corral.json
FINE: Going to write /Users/ordepdev/Work/corral/tmp/lock.json
INFO: Writing /Users/ordepdev/Work/corral/tmp/lock.json
INFO: init: created: tmp

Solves #23.

This change allow us to provide a directory that create and load the
`corral.json` file. It affects most commands as they mostly need to load
the file before doing their job. So, from now on, we can pass the
`--path "path/to/the/bundle/dir/" argument.

Solves ponylang#23.
@ordepdev
Copy link
Contributor Author

So, I took the liberty to include the path parameter in the context with two possible values, either the provided path or the default one, Path.cwd(). It makes things so easy after calling the corresponding command, we just need to use ctx.path everywhere we want to use it.

@SeanTAllen SeanTAllen requested review from cquinn and Theodus November 18, 2019 19:11
@SeanTAllen
Copy link
Member

@cquinn you are the subject matter expert. Can you give this a review?

@SeanTAllen
Copy link
Member

@ordepdev thoughts on tests that can be added with this change?

@ordepdev
Copy link
Contributor Author

ordepdev commented Nov 18, 2019

@SeanTAllen I can add tests for creating the corral.json file with a custom path.

Something like this maybe:

class iso _TestCreateBundleInCustomPath is UnitTest
  fun name(): String => "bundle/create-in-custom-path"

  fun apply(h: TestHelper) ? =>
    let log = Log(LvlFine, h.env.err, LevelLogFormatter)
    let expected = FilePath(h.env.root as AmbientAuth, "tmp")?
    match BundleFile.create_bundle(h.env, "tmp", log)
    | let bundle: Bundle =>
      h.assert_eq[String](expected.path, bundle.dir.path)
    end

@ordepdev ordepdev force-pushed the add-configurable-path branch from 9fad5fd to b650da7 Compare November 18, 2019 22:41
corral/bundle/bundle.pony Outdated Show resolved Hide resolved
corral/cmd/cmd_info.pony Show resolved Hide resolved
corral/main.pony Outdated Show resolved Hide resolved
corral/main.pony Outdated Show resolved Hide resolved
@ordepdev ordepdev requested a review from cquinn November 19, 2019 19:12
corral/bundle/bundle.pony Outdated Show resolved Hide resolved
corral/main.pony Outdated Show resolved Hide resolved
@SeanTAllen SeanTAllen added the changelog - added Automatically add "Added" CHANGELOG entry on merge label Nov 21, 2019
@SeanTAllen
Copy link
Member

@ordepdev do you want to squash this and push so you can control the commit message or shall I squash and merge?

@ordepdev
Copy link
Contributor Author

You can squash and merge with the first commit message.

@SeanTAllen SeanTAllen merged commit 49a20d1 into ponylang:master Nov 21, 2019
@SeanTAllen
Copy link
Member

Thanks, @ordepdev.

github-actions bot pushed a commit that referenced this pull request Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - added Automatically add "Added" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants