Skip to content

Consolidate all File Path node under Import/Export Category - #7755

Closed
fanwgwg wants to merge 5 commits into
DynamoDS:LibraryReorgfrom
fanwgwg:reorg-filepath
Closed

Consolidate all File Path node under Import/Export Category#7755
fanwgwg wants to merge 5 commits into
DynamoDS:LibraryReorgfrom
fanwgwg:reorg-filepath

Conversation

@fanwgwg

@fanwgwg fanwgwg commented Apr 6, 2017

Copy link
Copy Markdown
Contributor

Purpose

DYN-719 Consolidate all File Path node under Import/Export Category.

Old Nodes:
FilePath > ChangeExtension
FilePath > Combine
FilePath > DirectoryName
FilePath > Extension
FilePath > FileName
FilePath > HasExtension

New Nodes:
File.ChangePathExtension
File.CombinePath
File.DirectoryName
File.FileExtension
File.FileName
File.FileHasExtension

capture

Declarations

Check these if you believe they are true

  • The code base 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.

FYIs

@riteshchandawar @Randy-Ma @yeexinc

@fanwgwg fanwgwg mentioned this pull request Apr 6, 2017
Closed
7 tasks
@fanwgwg fanwgwg changed the title move nodes from file to importexport Consolidate all File Path node under Import/Export Category Apr 6, 2017
@User-Zhaoyang

User-Zhaoyang commented Apr 6, 2017

Copy link
Copy Markdown
Contributor

@fanwgwg
Great! I am a bit curious about why there are so many changes in the resx file?

@fanwgwg

fanwgwg commented Apr 6, 2017

Copy link
Copy Markdown
Contributor Author

@Randy-Ma I'm not quite sure either. After I did the renaming in the resource file and save it, some image entries got a space prepended in front.

@User-Zhaoyang

User-Zhaoyang commented Apr 7, 2017

Copy link
Copy Markdown
Contributor

One other thing is that it will be good if you can add some test cases for migration from old nodes to new nodes.

@yeexinc yeexinc mentioned this pull request Apr 7, 2017
Merged
7 tasks
@fanwgwg

fanwgwg commented Apr 7, 2017

Copy link
Copy Markdown
Contributor Author

@Randy-Ma I've just added a testcase for the migration.

@User-Zhaoyang

User-Zhaoyang commented Apr 7, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks a bunch!

@User-Zhaoyang User-Zhaoyang added LGTM and removed LGTM labels Apr 7, 2017
@User-Zhaoyang

User-Zhaoyang commented Apr 12, 2017

Copy link
Copy Markdown
Contributor

@fanwgwg
One thing is that the category should be "Import/Export" instead of "ImportExport". Right?

@fanwgwg

fanwgwg commented Apr 12, 2017

Copy link
Copy Markdown
Contributor Author

@Randy-Ma I've just updated the code so that it will be shown as Import/Export on UI.

capture

@fanwgwg fanwgwg mentioned this pull request Apr 12, 2017
Merged
@fanwgwg

fanwgwg commented Apr 12, 2017

Copy link
Copy Markdown
Contributor Author

Close this because of the duplicate #7776

@fanwgwg fanwgwg closed this Apr 12, 2017
@fanwgwg
fanwgwg deleted the reorg-filepath branch April 24, 2017 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fanwgwg @User-Zhaoyang