Skip to content

adjusted the repo structure to get it ready for including azure pipeline task #28

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

Merged
merged 4 commits into from
Aug 8, 2023

Conversation

zaowang-ms
Copy link
Contributor

This is the pull request to include azure pipeline task as discussed.

@zaowang-ms zaowang-ms added the enhancement New feature or request label Jul 17, 2023
@zaowang-ms zaowang-ms self-assigned this Jul 17, 2023
@ornj
Copy link
Member

ornj commented Jul 17, 2023

This PR is a bit more than adjusting the repo structure. I recommend sticking to the structure refactor so that can get approved with little to no controversy and then opening a new PR to add the Azure Pipeline. That way we can have the appropriate people review it without getting distracted by the refactor.

@amudukutore I think we need to add doc ops and an engineer with Typescript experience (@bheftel ?) to get @zaowang-ms's work reviewed.

@zaowang-ms
Copy link
Contributor Author

This PR is a bit more than adjusting the repo structure. I recommend sticking to the structure refactor so that can get approved with little to no controversy and then opening a new PR to add the Azure Pipeline. That way we can have the appropriate people review it without getting distracted by the refactor.

@amudukutore I think we need to add doc ops and an engineer with Typescript experience (@bheftel ?) to get @zaowang-ms's work reviewed.

Good suggestion. Let's have a follow up discussion as Ashok mentioned and then I can adjust as what we come up with

@amudukutore
Copy link
Contributor

@zaowang-ms - agreed with suggestion from @ornj on refactoring the PR into 2 separate ones - one for the restructure (which we can review between Steve and myself) and one for the rest of the Typescript (which we can request @bheftel to review).

@zaowang-ms
Copy link
Contributor Author

@zaowang-ms - agreed with suggestion from @ornj on refactoring the PR into 2 separate ones - one for the restructure (which we can review between Steve and myself) and one for the rest of the Typescript (which we can request @bheftel to review).

Thanks for taking a look. Good idea, I'll proceed with your suggestions and close this PR.

@zaowang-ms
Copy link
Contributor Author

@ornj @amudukutore Instead of close this current PR, I adjusted the files and make this pr only for restructuring. Now the changes should be easier to review. For adding the additional azure_pipelines, created a separate PR here #29

@zaowang-ms zaowang-ms changed the title adjusted the repo structure to include azure pipeline task adjusted the repo structure to get it ready for including azure pipeline task Aug 2, 2023
Copy link
Member

@ornj ornj left a comment

Choose a reason for hiding this comment

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

Nit, there are 3 styles for filenames now:

  • kebab-case
  • snake_case
  • camelCase

Suggest we choose one and we stick with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants