Add no revision option #474
Conversation
|
Hello @lenovouser, Thanks for your contribution. I'm curious what your motivation is for this feature. Linking to the master branch would cause the documentation links to break over time. |
|
Also, I'd suggest that if we were to commit to this feature, that we should do |
|
I would need this because I have the docs in my git repository and force them being generated with pre-commit hooks. Right now it always uses the latest hash which is kind of annoying because a ton of files get changed each time which clutters the commit view. You don't really see what was changed in the actual code because of all the |
| this.branch = out.stdout.replace('\n', ''); | ||
| } | ||
| } else if (this.revision) { | ||
| this.branch = this.revision; |
There was a problem hiding this comment.
Is there a better name we can think of than revision? What's the git correct name?
Also, what if we just made this.branch = revision and only run the if instead of requiring an if else? Do we need both pieces of information?
There was a problem hiding this comment.
Not sure, maybe branch?
There was a problem hiding this comment.
If we would take branch, should we also use --branch instead of --revision?
There was a problem hiding this comment.
Yes, we'll definitely make it consistent 😄 I'm not convinced on branch either, but it is what was previously used I suppose - even though it was pointing to a ref. Which reminds me, does --git-ref make more sense then? And then reference in the code?
There was a problem hiding this comment.
Hm. I mean it's already declared as branch in src/lib/converter/plugins/GitHubPlugin.ts#L33. Basically it can be a branch or a revision and until now it was declared as branch first and when it finds a git repository it replaces it with the revision. I don't mind reference either but branch or revision feels more natural to me.
(revision actually makes the most sense to me because a branch can be a revsion and well a revision is a revision 😆 - branch can just be a branch and not a revision from a logical point of view)
There was a problem hiding this comment.
If you feel revision more accurately reflects what it is, let's use that. I agree that branch is the least useful. As for the CLI flag, can we maybe do --git-revision to make it less ambiguous? I feel like any of revision, reference or branch without context won't make a ton of sense.
There was a problem hiding this comment.
Yes, sounds good to me. I'll use --gitRevision though since all the other flags are camelcase too, alright?
There was a problem hiding this comment.
Sounds good, thanks! Sorry, I just manage issues and PRs here and can never remember all the parts of the project, but you are right 👍
There was a problem hiding this comment.
Done, no problem. Do you want me to squash this?
There was a problem hiding this comment.
It's all good, thanks 👍 I'll squash when I merge it.
| @@ -6,6 +6,8 @@ import {Component, ConverterComponent} from '../components'; | |||
| import {BasePath} from '../utils/base-path'; | |||
| import {Converter} from '../converter'; | |||
| import {Context} from '../context'; | |||
| import {Option} from '../../utils/component'; | |||
| import {ParameterType} from '../../utils/options/declaration'; | |||
|
|
|||
| // This should be removed when @typings/shelljs typings are updated to the shelljs version being used | |||
There was a problem hiding this comment.
Might be a good time to remove manual shelljs typing if possible. Or we could do this in a separate PR.
There was a problem hiding this comment.
Definitely leave it for a different PR 😄
There was a problem hiding this comment.
Yeah, I have that already in a different branch. Will create a PR once this is merged.
No description provided.