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

bootstrapping swagger to .net sdk tools #5304

Closed
wants to merge 3 commits into from
Closed

bootstrapping swagger to .net sdk tools #5304

wants to merge 3 commits into from

Conversation

shahabhijeet
Copy link
Member

Set of files required to kick off swagger to sdk

@AutorestCI
Copy link

AutorestCI commented Mar 1, 2019 •
edited
Loading

Automation for azure-sdk-for-ruby

Unable to detect any generation context from this PR.

@AutorestCI
Copy link

AutorestCI commented Mar 1, 2019 •
edited
Loading

Automation for azure-sdk-for-js

Unable to detect any generation context from this PR.

@AutorestCI
Copy link

AutorestCI commented Mar 1, 2019 •
edited
Loading

Automation for azure-sdk-for-node

Unable to detect any generation context from this PR.

@AutorestCI
Copy link

AutorestCI commented Mar 1, 2019 •
edited
Loading

Automation for azure-sdk-for-go

Unable to detect any generation context from this PR.

@AutorestCI
Copy link

AutorestCI commented Mar 1, 2019 •
edited
Loading

Automation for azure-sdk-for-python

Unable to detect any generation context from this PR.

@AutorestCI
Copy link

AutorestCI commented Mar 1, 2019 •
edited
Loading

Automation for azure-sdk-for-java

Unable to detect any generation context from this PR.

@shahabhijeet shahabhijeet requested a review from salameer March 1, 2019 07:11
sergey-shandar
sergey-shandar reviewed Mar 1, 2019
View reviewed changes
Copy link
Contributor

@sergey-shandar sergey-shandar left a comment

Choose a reason for hiding this comment

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

I know that this is just a draft but I would propose to download nuget.exe instead of adding it to source control.

@shahabhijeet
Copy link
Member Author

shahabhijeet commented Mar 1, 2019 •
edited
Loading

Yes I agree with you, if we can download it, that will be the best option. Let me see if we can eliminate one binary out of our PR.
The worst case due to time constraint, we would like to start integrating this and trust me I would like to remove my static dependency off binaries as much as possible.

deleting nuget.exe binary. Updating configs to locally restore pacakg…
a5f784a 
…es. Adding the directory that will be used in restoring packages locally to .gitignore
@sergey-shandar sergey-shandar assigned ghost Mar 2, 2019
ghost
ghost approved these changes Mar 4, 2019
View reviewed changes
stostools/Directory.Build.props Outdated Show resolved Hide resolved
stostools/Directory.Build.props Outdated Show resolved Hide resolved
stostools/NuGet.Config Outdated Show resolved Hide resolved
@shahabhijeet
Copy link
Member Author

@salameer can you sign off on this PR.

@salameer
Copy link
Member

salameer commented Mar 5, 2019

@daschult are we waiting for something to merge this?

@sergey-shandar why is this check not completed yet? is this related to the similar issues we've been having on other PRs?

@ghost
Copy link

ghost commented Mar 5, 2019

@salameer @shahabhijeet mentioned that he didn't want to merge this PR until you approved it. That's the only reason I haven't merged it already.

salameer
salameer requested changes Mar 5, 2019
View reviewed changes
Copy link
Member

@salameer salameer left a comment

Choose a reason for hiding this comment

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

@shahabhijeet Can you please add your new stostools under the "Scrpts folder" I would like to avoid having more folders at the top level of the repo

@sergey-shandar
Copy link
Contributor

@shahabhijeet It looks like Travis-CI is stuck. I don't have a link to Travis-CI job so the easiest way would be to make a dummy commit.

@shahabhijeet
Copy link
Member Author

shahabhijeet commented Mar 6, 2019 •
edited
Loading

@salameer will this work?
\scripts\stostools\ < all files under this folder >

@shahabhijeet
Copy link
Member Author

@salameer @sergey-shandar is travis CI waiting for anything else? I added the commit that should have done the trick?

@shahabhijeet shahabhijeet marked this pull request as ready for review March 7, 2019 00:41
@shahabhijeet
Copy link
Member Author

Closing this one and opening a new one for travis to kick in #5334

@shahabhijeet shahabhijeet mentioned this pull request Mar 7, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sergey-shandar sergey-shandar sergey-shandar left review comments

@salameer salameer salameer requested changes

Assignees

@sergey-shandar sergey-shandar

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@shahabhijeet @AutorestCI @salameer @sergey-shandar