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

Move EngSys scripts and tools into the eng folder #15409

Closed
4 tasks
RickWinter opened this issue Aug 30, 2021 · 4 comments
Closed
4 tasks

Move EngSys scripts and tools into the eng folder #15409

RickWinter opened this issue Aug 30, 2021 · 4 comments
Assignees
Labels
Central-EngSys This issue is owned by the Engineering System team. Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. feature-request This issue requires a new behavior in the product in order be resolved.

Comments

@RickWinter
Copy link
Member

RickWinter commented Aug 30, 2021

There are currently yaml files and various scripts at the root of the repo. These should be moved into the Eng folder.

All of these moves should be coordinated with mgmt sdk to avoid disruption to their process.

In the root folder of repo:

  • Move azure-pipelines.yml into eng folder
  • Move tools folder under eng
  • Move all *.sh files to eng folder
  • Move all *.json files to eng folder
@RickWinter RickWinter added Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. feature-request This issue requires a new behavior in the product in order be resolved. labels Aug 30, 2021
@benbp
Copy link
Member

benbp commented Sep 2, 2021

Typing up some notes here. CC @lirenhe

Simple fixes

  • The following files either have no references, already have copies in eng, or are a simple reference fix, assuming there are no non-obvious dependencies on them.
    • initScript.sh, swagger_to_sdk_config.json, generate_options.json, rungas.sh, azure-pipelines.yml

More involved

  • All the module references for the tools projects (e.g. github.com/Azure/azure-sdk-for-go/tools/internal) will need to be updated.
    • There are a couple ways to do this. The one I'm leaning towards is to do a git move of the tools directory to eng/tools, then do a mass replace of the module paths. Once that's checked in, I can do a follow-on PR updating all the go.sum files. It would be slightly less disruptive to copy the entire directory and have both checked in during the migration so that both module paths are valid. That approach would remove the git history of the files, which is not ideal.
    • This all assumes that there is nothing outside of this repo taking dependencies on this modules (either way, I wouldn't say they're "supported"). The only references I can find are for the auto-generated by comments, which won't be breaking: https://grep.app/search?q=azure-sdk-for-go/tools
  • The auto-generated by messages in the models.go files in profiles will need to be updated, but I assume this could just happen on the next auto-gen pass.

@ArcturusZhang
Copy link
Member

Some correction:

  1. Despite we already have a swagger_to_sdk_config.json in eng directory, but one of them is for track 1 SDK (the one in root directory), and the other is for track 2 SDK. Therefore we need to keep them both.
  2. We have some external reference for these two configurations here, we need to change those accordingly after we have done the refactor.
  3. rungas.sh as far as I know this is never used since we moved to azure-pipelines. We can remove this file. @jhendrixMSFT to confirm.

I propose that we could do the move of tools in two steps:

  1. We move the internal module to eng/tools and change all the references accordingly. Replace it using relative path to make sure everything could work
  2. We move all the tools to eng/tools. In this step we do not need to do any code change, just remove the replace clause and use the new internal module.

@jhendrixMSFT
Copy link
Member

Yeah rungas.sh can be deleted.

@benbp
Copy link
Member

benbp commented Sep 2, 2021

  1. Despite we already have a swagger_to_sdk_config.json in eng directory, but one of them is for track 1 SDK (the one in root directory), and the other is for track 2 SDK. Therefore we need to keep them both.
  2. We have some external reference for these two configurations here, we need to change those accordingly after we have done the refactor.

Looking at other repos, I realize this is not an uncommon pattern, so I don't see a need right now to delete the root swagger config for consistency reasons right now.

  1. We move the internal module to eng/tools and change all the references accordingly. Replace it using relative path to make sure everything could work
  2. We move all the tools to eng/tools. In this step we do not need to do any code change, just remove the replace clause and use the new internal module.

Sounds good. I have a PR up for step 1 at #15460.

@benbp benbp added the Central-EngSys This issue is owned by the Engineering System team. label Oct 25, 2021
@benbp benbp closed this as completed Oct 25, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Central-EngSys This issue is owned by the Engineering System team. Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system. feature-request This issue requires a new behavior in the product in order be resolved.
Projects
None yet
Development

No branches or pull requests

4 participants