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

rename some long documentation files and add more logging to rename command. #13894

Merged
merged 2 commits into from
Apr 12, 2023

Conversation

mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Apr 11, 2023

Purpose

https://jira.autodesk.com/browse/DYN-5508

TODO:

  • file followups

some doc files have names that are too long - this PR does the following:

  • renames the known ones that have caused issues for host installers.
  • as a byproduct - renames MOST of the related support files. (images/dyns)
  • keeps the existing files - these will still need to be excluded by some hosts as other hosts have included them.
  • renames all overloads with the same names as these nodes.
  • adds a verbose options to the rename command - this will log some warnings and generate a rename log, which I have committed - if we continue to use this rename process, I think we should append any new logs to the existing one - the command does this already if a log is found.

an example of the kind of warning that will be generated:

PS C:\Users\kirschm> cd C:\Users\kirschm\source\repos\mjkkirschner\Dynamo\src\Tools\NodeDocumentationMarkdownGenerator\bin\Debug
PS C:\Users\kirschm\source\repos\mjkkirschner\Dynamo\src\Tools\NodeDocumentationMarkdownGenerator\bin\Debug> .\NodeDocumentationMarkdownGenerator.exe rename -d "C:\Users\kirschm\source\repos\mjkkirschner\Dynamo\doc\distrib\NodeHelpFiles\to rename" -v -m 10

 _______
/       \
$$$$$$$  | __    __  _______    ______   _____  ____    ______
$$ |  $$ |/  |  /  |/       \  /      \ /     \/    \  /      \
$$ |  $$ |$$ |  $$ |$$$$$$$  | $$$$$$  |$$$$$$ $$$$  |/$$$$$$  |
$$ |  $$ |$$ |  $$ |$$ |  $$ | /    $$ |$$ | $$ | $$ |$$ |  $$ |
$$ |__$$ |$$ \__$$ |$$ |  $$ |/$$$$$$$ |$$ | $$ | $$ |$$ \__$$ |
$$    $$/ $$    $$ |$$ |  $$ |$$    $$ |$$ | $$ | $$ |$$    $$/
$$$$$$$/   $$$$$$$ |$$/   $$/  $$$$$$$/ $$/  $$/  $$/  $$$$$$/
          /  \__$$ |
          $$    $$/
           $$$$$$/

looking for dynamo core assemblies in ..\..\..\..\..\bin\AnyCPU\Debug
Autodesk.DesignScript.Geometry.CoordinateSystem.Transform(coordinateSystem, fromCoordinateSystem, contextCoordinateSystem).md references Autodesk.DesignScript.Geometry.CoordinateSystem.Transform(CS, fromCS, contextCS)_img.png, but C:\Users\kirschm\source\repos\mjkkirschner\Dynamo\doc\distrib\NodeHelpFiles\to rename\Autodesk.DesignScript.Geometry.CoordinateSystem.Transform(CS, fromCS, contextCS)_img.png will not be renamed as it does not contain the same basename as the md file. Manually rename this file and the reference.
Autodesk.DesignScript.Geometry.CoordinateSystem.Transform(CS, fromCS, contextCS)_img.png was not found in the rename log

See rename_log.txt for the log.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

Rename some documentation files so they can be shipped with dynamo.

Reviewers

@pinzart90
Copy link
Contributor

not sure I understand this fully:

renames the known ones that have caused issues for host installers.
keeps the existing files - these will still need to be excluded by some hosts as other hosts have included them.

So you renamed some content (.dyn) from some long name to a GUID based name. These renamed files have been added to git.
The old (long named) files are still in git (not removed) because host integrators still expect them ? So now we have duplicate content ?

@sm6srw
Copy link
Contributor

sm6srw commented Apr 12, 2023

not sure I understand this fully:

renames the known ones that have caused issues for host installers.
keeps the existing files - these will still need to be excluded by some hosts as other hosts have included them.

So you renamed some content (.dyn) from some long name to a GUID based name. These renamed files have been added to git. The old (long named) files are still in git (not removed) because host integrators still expect them ? So now we have duplicate content ?

Some hosts have already included these longs file names in their installers so we need to keep them around until next RTM (requirement for the old patch installer, we can't remove files). This is hopefully the last time we need to do this (keep the files around, that is). The new patch installer does not have this limitation.

In general, I think I am on Mike's side that we should find a better way to do this. Maybe check with the localization team and see what they currently support that may fit our use case.

Copy link
Contributor

@sm6srw sm6srw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

if (Verbose)
{
//do a scan for support files which do not share the base name.
foreach (var nonMDFile in allNonMDFiles)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you did find support files without a base file? What that caused by someone deleting the basename but not the supportfiles? I can't remember seeing this before.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a single support file that is referenced from one of the renamed files, but its argument names are different, so it does not get renamed.

See my sample output for the example.

I didn't end up changing it as the name seemed short enough.

internal static void HandleRename(RenameOptions opts)
{

Verbose=opts.Verbose;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding Verbose support.

{
foreach (var mdfile in allMdFiles)
{
//md file contains a reference to the support file, but the names are not the same.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice consistency check. I don't remember seeing that case neither. But it was much fewer files when i did this.

@mjkkirschner
Copy link
Member Author

thanks for the review- I will merge after the tests pass.
will file two followups

  1. remove duplicate files for GL25
  2. spike on delivering builtin node docs in resource assembly to avoid long path issue.

@mjkkirschner mjkkirschner merged commit e6cba73 into DynamoDS:master Apr 12, 2023
@mjkkirschner mjkkirschner deleted the longfilepaths branch April 12, 2023 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants