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

Add Dependency Info #3115

Merged
merged 7 commits into from
Mar 17, 2020
Merged

Add Dependency Info #3115

merged 7 commits into from
Mar 17, 2020

Conversation

michael-hawker
Copy link
Member

Add info in readme for building the toolkit repo and dependencies required.

PR Type

What kind of change does this PR introduce?

  • Documentation content changes

Add info in readme for building the toolkit repo and dependencies required.
@ghost
Copy link

ghost commented Jan 29, 2020

Thanks for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@michael-hawker
Copy link
Member Author

michael-hawker commented Jan 30, 2020

@azchohfi we noticed that 4.6.2 was called out for Marcus in a build step too. Could we have a dependency mismatch somewhere on both 4.6.1 and 4.6.2?

What is the config line that pulls that in, where does that live?

readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
Copy link
Member

@clairernovotny clairernovotny left a comment

Choose a reason for hiding this comment

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

LGTM, the only suggestion I would have is to look into moving the CI package feed to Azure Artifacts instead of MyGet

readme.md Outdated Show resolved Hide resolved
@michael-hawker
Copy link
Member Author

@clairernovotny Thanks Claire! I know there's also GitHub packages as well, maybe it's best we open a separate issue to track that discussion and understand the pros/cons of each solution and the effort involved in moving over to them?

@michael-hawker
Copy link
Member Author

Thanks @azchohfi I'll update this guidance to call out the different SDKs needed for building the toolkit vs. the sample app. I guess ideally we should converge to the same one, I just know we're not using anything specific in the newer SDKs currently.

@ghost
Copy link

ghost commented Mar 16, 2020

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

@michael-hawker
Copy link
Member Author

@azchohfi this look better for now until we update and converge later?

Copy link
Contributor

@azchohfi azchohfi left a comment

Choose a reason for hiding this comment

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

That is perfect!

readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants