Add the "add" comand.

[hibnico]

The 'install' "add" command takes care of modifying properly the bundle.json, even create one if none exists. And after the modification of the bundle.json it is launching the fetch.

[hibnico]

I just saw that I have missed the support for 'subdir' for github. So it needs a little more work.

[hibnico]

PR updated, but now needs https://github.com/ponylang/ponyc/pull/1115 to be accepted

[jemc]

FWIW I think I'm not a big fan of calling this install, as I find it confusing in this context. When I think of installing something, I think of installing it globally on a system, and having it become integrated with that system.

I know npm uses the term install --save to mean this same thing, but I don't find it to be very intuitive here.

I would suggest calling it add.

[jemc]

Also, there are some style issues where the new code conflicts with the style of the current code, leaving a mixed style as a result. I will do a full review later (pending acceptance of your PR to ponyc), but some general style things I noticed in my quick glance through this:

• all method, local, and parameter names should be in snake_case, not camelCase.
• try to match the existing style for where whitespace and/or empty lines should and shouldn't be.
• the source file names should correspond to the principle type in each file, according to the same rules now officially adopted by the ponyc repo.
  • I saw you split bundle_dep.pony into multiple source files - this in itself isn't problematic, but the principal type names still need to be consistent with the file names, and if there is more than one principal type in a single file they need to share a common prefix. This part is really important to me, as I find it essential to good code organization in Pony. It needs to be super obvious where each type is located.
  • If you need to, don't be afraid to push bundle_dep stuff into a subpackage dedicated to expressing bundle dep entries. This might be the best option for avoiding super long type names.

[hibnico]

• install vs add: as you pointed I chose 'install' because npm uses it. But I agree with it doesn't reflect nicely what the command is doing. Let's call it add.
• snake vs camel: I have some personal preference for camel, and I think I authorized myself to use it because I saw some camel cased local variables. But I am versatile, I'll use the snake style, no problem.
• whitespace, empty lines: I tried to do my best because I don't know the exact rules. Since Pony have the same structural style as Python, I tried to applied Python's one (basically one line between functions, 2 between classes). Or it is different ? Also, I use Atom and it is removing spaces on blank lines. Trying to make the PR clean I reverted these removes from the existing code. Should I let them in ?
• file names: I agree with you, I'll follow ponyc guidelines.
• subpackages: since splitting into different packages has an effect on visibility of the classes, I prefer to contain as much as possible everything in one package and keep some classes private, like _SubdirNameGenerator for instance. I care of the API because even if it is intended to be "just" an executable, we never know the integration that might happens later. And if I would split, I would split for each kind of repo, because as soon as Pony can support that, they could be plugins. So for now I would keep it that way, which may be revisited later.

[jemc]

I saw some camel cased local variables

Can you point out where these are? I don't know of any in the codebase, but if they are there, they must have been contributed by someone else with me not catching it in review. If they're there, they should be changed.

I use Atom and it is removing spaces on blank lines. Trying to make the PR clean I reverted these removes from the existing code.

Yeah, a lot of folks have editors that make this assumption, unfortunately. If you want to fix it the easy way, you can run scripts/fix-whitespace stable/*.pony before committing to restore the expected whitespace.

[hibnico]

Can you point out where these are? I don't know of any in the codebase, but if they are there, they must have been contributed by someone else with me not catching it in review. If they're there, they should be changed.

I haven't found it in the current master, so it must be me which has modified some code and didn't remember it. So I saw myself changing the style thinking it was somebody else, looks like I have some double personality issue ! :p

[jemc]

Sorry, I forgot this was sitting stale - I forgot to come back to it after the options package was updated.

There are some name changes I'm not wild about here, like ProjectRepo and *ProjectRepo replacing BundleDep and BundleDep*. There are also some remaining style points to clean up.

However, rather than explaining in detail all the changes I'd want made, I think it's easier just to make them. I'm going to work on a branch that includes your commits, then merge that branch when it's done. If I use your branch as the base of mine (as I plan to do), GitHub should show this PR as being merged when I merge my branch.

[hibnico]

I introduced the concept of project repository as a facade to several kind of implementations of sources hosting. So the core of Pony Stable doesn't know the specifics of how sources are hosted in github. And adding support for a new repo, like bitbucket for instance, will be about implementing an interface and referencing it in the factory; so it would not be about touching a lot of files, and it would be typed so there is no way of missing to implement a command. I wanted to properly document it to better explain it but I forgot. I should have.

I also wanted to add a new command, "help", to document inline the commands. See https://github.com/nlalevee/pony-stable/tree/help/stable I don't mind much rewriting it. But should I still use your style of implementing commands ? Even if it will duplicate the switch on the kind of repo ?

[hibnico]

I just listened to the Pony Sync of the 7th of September. There was a very interesting discussion about how to correctly type things. It was hard to grasp every detail of it, especially at the end of the talk with Sylvan, about the "seek" exemple. But just to let you know that I have been working on yet another command, named "resolve". It still needs some work, and would also need some discussion to see if it really fits Pony stable philosophy. This feature in itself is off topic here, but the implementation needs the output of the git command, so it needs to use the process package, so it needs the AmbientAuth. So in order to not pollute the dependency API with specific things for git, the code handling git has to be an instance which would be constructed with the proper AmbientAuth. So even if parsing arguments or json and outputting the help are independent of the instance, that new resolve command will not. I don't know how this would affect the design you were discussing with Sylvan, but I just wanted to let you know that I would need an instance at some point.