-
Notifications
You must be signed in to change notification settings - Fork 832
feat(import external docs): allow contributor url for import #20166
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
feat(import external docs): allow contributor url for import #20166
Conversation
…repositories chore: Updated pwsh alias to recommended pwsh syntax chore: Added and implement custom url parameter to ctor of script, to enable contributor url acceptance of the script when its not listed at the depending uno repo so far
…le testing it chore: update from alias usage to recommended full qualified functions # Conflicts: # build/cSpell.json
…and use PSScriptRoot to ensure correct path
…ectly in future, did not last run without
…null if not in use
|
@kazo0 @agneszitte tryed to fix the import script, but this need a last bit of help to be ready |
|
@jeromelaban I'm not sure if this is something we'd want to change? However, what would be the easiest way for @DevTKSS to be able to test their docs changes locally given that they are on a fork? I was imagining that they would be able to change this line: uno/doc/import_external_docs.ps1 Line 24 in c753d15
uno/doc/import_external_docs_test.ps1 Lines 9 to 20 in c753d15
|
|
@kazo0 so like do it in a two step thing? but that would potentially fail because after the import, the docfx and dotnet-serve command is called right after, so this will defintly fail. And currently, when I maybe did already run this once (and its telling me the branch with the sha of my fork is not on the uno-origin (of corse not ;) ) ) then on second run where I maybe would have added another commit to fix links, then it is failing because the repo (no matter which) is already existing at external, directory, so maybe this part of the problem would require a clean up before checking out again? meaning the checkouts of the uno-origin, non fork external imports if unclear worded. |
|
currently that partial import would fail also because the docfx would attempt to resolve all links no matter if it might be the first of two import steps, which is of corse not possible until all externals are there. Or do you see another possible workaround to tell it to not to this or run a second run with the other url? thats just what I tryed to introduce with this, to provide the other url(s) which would essentially just need one url, because mostly we only have one account at the time. |
| mkdir articles\external -ErrorAction Continue | ||
| } | ||
|
|
||
| Push-Location articles\external |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's best to avoid cosmetic changes with functional changes. Could you make a PR only with the custom git change?
GitHub Issue (If applicable): closes #
PR Type
What kind of change does this PR introduce?
What is the current behavior?
What is the new behavior?
PR Checklist
Please check if your PR fulfills the following requirements:
Screenshots Compare Test Runresults.Other information
Problem this need a last bit of help with:
after running the main script, when it returns to the known
_Test.ps1the Script executing path does not match coming from the Location change in the main script for importing the externals.thats most likly just a quick fix but I do not know.
causing problems for most of my docs PR, e.g.:
Internal Issue (If applicable):