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.

Sign up for GitHub

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

Jump to bottom

more namespace move for transforms #1457

Merged
merged 5 commits into from
Oct 30, 2018
Merged

more namespace move for transforms #1457

merged 5 commits into from
Oct 30, 2018

Conversation

sfilipi
Copy link
Member

@sfilipi sfilipi commented Oct 30, 2018

Addresses part of #1318

@sfilipi sfilipi requested a review from Zruty0 October 30, 2018 19:24
@sfilipi sfilipi self-assigned this Oct 30, 2018
@sfilipi sfilipi added the API Issues pertaining the friendly API label Oct 30, 2018
Ivanidzo4ka
Ivanidzo4ka reviewed Oct 30, 2018
View reviewed changes
src/Microsoft.ML.Transforms/TermLookupTransform.cs Outdated
@@ -22,7 +21,7 @@
[assembly: LoadableClass(TermLookupTransform.Summary, typeof(TermLookupTransform), null, typeof(SignatureLoadDataTransform),
"Term Lookup Transform", TermLookupTransform.LoaderSignature)]

namespace Microsoft.ML.Runtime.Data
namespace Microsoft.ML.Transforms.Text
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka Oct 30, 2018 •
edited by sfilipi
Loading

Choose a reason for hiding this comment

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

It should be in same namespace as Term transform.
Which is right now is Microsoft.ML.Transforms.Categorical. #Resolved

Ivanidzo4ka
Ivanidzo4ka approved these changes Oct 30, 2018
View reviewed changes
Copy link
Contributor

@Ivanidzo4ka Ivanidzo4ka left a comment

Choose a reason for hiding this comment

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

:shipit:

JRAlexander
JRAlexander approved these changes Oct 30, 2018
View reviewed changes
Copy link

@JRAlexander JRAlexander left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. :shipit: (after build is successful)

shmoradims
shmoradims approved these changes Oct 30, 2018
View reviewed changes
Copy link

@shmoradims shmoradims left a comment

Choose a reason for hiding this comment

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

:shipit:

@sfilipi sfilipi merged commit e4c2fa0 into dotnet:master Oct 30, 2018
@sfilipi sfilipi deleted the namespaceReorg6 branch October 30, 2018 22:34
@ghost ghost locked as resolved and limited conversation to collaborators Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Ivanidzo4ka Ivanidzo4ka Ivanidzo4ka approved these changes

@JRAlexander JRAlexander JRAlexander approved these changes

@shmoradims shmoradims shmoradims approved these changes

@Zruty0 Zruty0 Awaiting requested review from Zruty0

Assignees

@sfilipi sfilipi

Labels
API Issues pertaining the friendly API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sfilipi @Ivanidzo4ka @JRAlexander @shmoradims