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

Removing x namespace from go/python/nodejs automation packages #6518

Merged
merged 3 commits into from Mar 17, 2021
Merged

Removing x namespace from go/python/nodejs automation packages #6518

merged 3 commits into from Mar 17, 2021

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Mar 12, 2021 •
edited

All of these were moved with git mv so that we preserve the history!

@stack72 stack72 requested a review from komalali March 12, 2021 17:27
@stack72 stack72 self-assigned this Mar 12, 2021
@stack72 stack72 changed the title Moving sdk/go/x/auto to sdk/go/auto Removing x namespace from go/python/nodejs automation packages Mar 12, 2021
@stack72 stack72 force-pushed the move-go-automation-api branch 2 times, most recently from 6d3b74a to d3a7a79 Compare March 12, 2021 19:40
komalali
komalali reviewed Mar 15, 2021
View reviewed changes
Copy link
Member

@komalali komalali left a comment

Choose a reason for hiding this comment

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

This looks 99% of the way there. I think we're just missing exporting automation from the top-level index.ts for nodejs.

@stack72
Copy link
Contributor Author

stack72 commented Mar 16, 2021

This looks 99% of the way there. I think we're just missing exporting automation from the top-level index.ts for nodejs.

Interesting - the original module didn't have the import of X so I didn't remove anything there - if this was needed wouldn't the tests fail?

@komalali
Copy link
Member

komalali commented Mar 16, 2021 •
edited

It's not "needed" like it would make the tests fail (since the tests are using relative imports), but it is needed so folks can do import {automation} from "@pulumi/pulumi" - it was probably an oversight that this was left out initially. If you look at the top-level index.ts it includes all of the other submodules.

Right now the only way to import from the automation module is to do something like import { InlineProgramArgs, LocalWorkspace } from "@pulumi/pulumi/automation"; which is inconsistent with the rest of our modules.

@stack72
Copy link
Contributor Author

stack72 commented Mar 16, 2021

It's not "needed" like it would make the tests fail (since the tests are using relative imports), but it is needed so folks can do import {automation} from "@pulumi/pulumi" - it was probably an oversight that this was left out initially. If you look at the top-level index.ts it includes all of the other submodules.

Right now the only way to import from the automation module is to do something like import { InlineProgramArgs, LocalWorkspace } from "@pulumi/pulumi/automation"; which is inconsistent with the rest of our modules.

gotcha!! Then it's a good thing to add :)

@stack72 stack72 requested a review from komalali March 16, 2021 17:16
komalali
komalali approved these changes Mar 16, 2021
View reviewed changes
Copy link
Member

@komalali komalali left a comment

Choose a reason for hiding this comment

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

:shipit:

@stack72 stack72 mentioned this pull request Mar 17, 2021
Closed
@stack72 stack72 merged commit 86dc543 into feature-3.0 Mar 17, 2021
@stack72 stack72 deleted the move-go-automation-api branch March 17, 2021 11:58
stack72 added a commit that referenced this pull request Mar 22, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
94626b7
stack72 added a commit that referenced this pull request Mar 24, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
bc1e73a
stack72 added a commit that referenced this pull request Mar 24, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
9668cd2
stack72 added a commit that referenced this pull request Mar 24, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
7972228
stack72 added a commit that referenced this pull request Mar 24, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
e102dcc
stack72 added a commit that referenced this pull request Mar 24, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
0c28031
stack72 added a commit that referenced this pull request Mar 31, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
28546d7
viveklak pushed a commit that referenced this pull request Apr 3, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
536f70d
stack72 added a commit that referenced this pull request Apr 5, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
848649f
stack72 added a commit that referenced this pull request Apr 5, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
982d7db
stack72 added a commit that referenced this pull request Apr 5, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
377ecf6
stack72 added a commit that referenced this pull request Apr 5, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
43627cf
stack72 added a commit that referenced this pull request Apr 7, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
7fbad80
stack72 added a commit that referenced this pull request Apr 13, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
051aaee
stack72 added a commit that referenced this pull request Apr 14, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
4019789
stack72 added a commit that referenced this pull request Apr 14, 2021
@stack72
Removing x namespace from go/python/nodejs automation packages (#6518)
3fad2e5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@komalali komalali komalali approved these changes

Assignees

@stack72 stack72

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@stack72 @komalali