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

Remove aliases for docfx without subcommand #35

Closed
pascalberger opened this issue Apr 29, 2017 · 7 comments
Closed

Remove aliases for docfx without subcommand #35

pascalberger opened this issue Apr 29, 2017 · 7 comments
Assignees
Milestone

Comments

@pascalberger
Copy link
Member

pascalberger commented Apr 29, 2017

Calling docfx docfx.json seems to be only a shortcut to docfx build docfx.json. Therefore it doesn't make sense to provide different aliases for them beside the DocFxBuild() aliases and we should return the DocFx() aliases.

@pascalberger
Copy link
Member Author

@reicheltp Would this be OK for you? Or am I missing a point and there is a difference between calling docfx docfx.json and docfx build docfx.json?

@pascalberger
Copy link
Member Author

@reicheltp #38 is a PR for marking the DocFx() alias as obsolete, but I would also be OK to completely remove it. What's your opinion about this?

@reicheltp
Copy link
Member

@pascalberger I think we should keep it obsolete for 1 release and than remove it.

pascalberger added a commit that referenced this issue May 2, 2017
@agc93
Copy link

agc93 commented May 8, 2017

Out of curiosity here, given that docfx.exe supports calling with no subcommands, and most of the "CLI-wrapping" Cake addins tend to match their respective exe syntaxes as a convention, why did we need to remove this alias?

Seems like both are a perfectly valid use case with DocFx() matching the behaviour of docfx.exe and DocFxBuild() matching the behaviour of docfx.exe build (which just so happen to be the same).

Just wondering.

@pascalberger
Copy link
Member Author

@agc93 The other way round: Whats the advantage of having aliases for calling docfx without a subcommand considered its very limited possibilites. As a consumer of the Cake.DocFx addin I would be confused why there's a DocFx() alias and a DocFxBuild() alias doing the same, but one providing much more possibilities which the other couldn't provide (eg. the recently introduced possibility to pass global metadata or previewing the built documentation).

The possibility to call docfx.exe without the build subcommand makes sense for a CLI for convinience, but not really in a Cake addin IMHO.

Additionally the current comments were quite missleading indicating that there was a functional difference between the two, which as far as DocFx documentation and my experience with it goes does not exist.

@agc93
Copy link

agc93 commented May 8, 2017

Understood and agreed re misleading documentation/comments, just seems odd to me to remove a whole alias purely because it's a shortcut (and arguably the most used invocation of docfx.exe).

As a consumer of docfx.exe, now looking to move to Cake, I could conceivably be confused by the fact that the command I've been using up until now (since even the Getting Started docs use the short form) is not available.

Certainly seems to me that the existing limited-capability DocFx() alias complemented by the more capable DocFxBuild() alias had the desired intention: matching docfx.exe's behaviour almost exactly:

Usage1: docfx <docfx.json file path> [-o <output folder path>]
Usage2: docfx <subcommand> [<args>]

in addition to matching the convention set by other addins of making "advanced" configuration opt-in where possible.

Like I said, just my $0.02 but seems odd to diverge from the underlying tool's behaviour (in a breaking change no less) for what seems like purely stylistic reasons?

@pascalberger
Copy link
Member Author

@agc93 I don't have necessarily a problem with keeping the alias and add clear documentation pointing the user to the DocFxBuild() aliases for advanced scenarios.

Still not sure what is less confusing to the user, though 😄. But probably this depends on the background. In your case you're already familiar with the CLI syntax. For new users I think it might still be confusing to have a DocFx() alias which can only be used for building, even though its name is function agnostic and which does the same as DocFxBuild(). Proper documentation might help in this case (eg. its already much clearer on http://cakebuild.net/dsl/docfx/ with the added CakeAliasCategoryAttribute that DocFx() and DocFxBuild() are similar).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants