Skip to content

Fix Folder Structure#1241

Merged
Aniruddh25 merged 44 commits intomainfrom
dev/aaronburtle/fixFolderStructure
Feb 28, 2023
Merged

Fix Folder Structure#1241
Aniruddh25 merged 44 commits intomainfrom
dev/aaronburtle/fixFolderStructure

Conversation

@aaronburtle
Copy link
Contributor

@aaronburtle aaronburtle commented Feb 17, 2023 •
edited
Loading

Why make this change?

Closes #1206

What is this change?

Move scripts, create_manifest_file.ps1, dab-smoke-test-script.ps1, and publish.ps1 into the scripts directory and rename ConfigGenerators to configGenerators to follow convention of lowercase starting letter. We also modify the build pipeline and scripts to account for new folder structure.

How was this tested?

Run against test suite.

Sample Request(s)

N/A

@seantleonard
Copy link
Contributor

seantleonard commented Feb 17, 2023

rename folders to follow convention of lowercase starting letter.

Which folder were renamed? GitHub change log only shows scripts being moved to scripts folder

@Aniruddh25
Copy link
Collaborator

Aniruddh25 commented Feb 22, 2023 •
edited
Loading

Should we flatten the src/Cli folder structure as part of this PR or in a separate one?

I think we can fit that into this PR as well, how should we flatten the src/Cli folder structure? You mean simply eliminating src and test within Cli, have it all just within Cli itself?

having both src files and test files within Cli itself would make it unorganized where both source files and test files are mixed up.

@aaronpowell, did you have a different proposal for flattening src/Cli?

how about we create a new folder for src/Cli.Tests?
src/Cli gets files that are currently under src/Cli/src
src/Cli.Tests gets files which are today under src/Cli/test

@aaronpowell
Copy link
Contributor

aaronpowell commented Feb 22, 2023

Should we flatten the src/Cli folder structure as part of this PR or in a separate one?

I think we can fit that into this PR as well, how should we flatten the src/Cli folder structure? You mean simply eliminating src and test within Cli, have it all just within Cli itself?

having both src files and test files within Cli itself would make it unorganized where both source files and test files are mixed up.

@aaronpowell, did you have a different proposal for flattening src/Cli?

how about we create a new folder for src/Cli.Test? src/Cli gets files that are currently under src/Cli/src src/Cli.Tests gets files which are today under src/Cli/test

My thought it what @aaronburtle suggests, as that would have the CLI folders fitting into the rest of the folder structure.

It'd see this:

/src
  /Auth
  /Cli
  /Cli.Tests
  /Config
  /Service
  /Service.GraphQLBuilder
  /Service.Tests

@Aniruddh25
Copy link
Collaborator

Aniruddh25 commented Feb 23, 2023

Should we flatten the src/Cli folder structure as part of this PR or in a separate one?

I think we can fit that into this PR as well, how should we flatten the src/Cli folder structure? You mean simply eliminating src and test within Cli, have it all just within Cli itself?

having both src files and test files within Cli itself would make it unorganized where both source files and test files are mixed up.
@aaronpowell, did you have a different proposal for flattening src/Cli?
how about we create a new folder for src/Cli.Test? src/Cli gets files that are currently under src/Cli/src src/Cli.Tests gets files which are today under src/Cli/test

My thought it what @aaronburtle suggests, as that would have the CLI folders fitting into the rest of the folder structure.

It'd see this:

/src
  /Auth
  /Cli
  /Cli.Tests
  /Config
  /Service
  /Service.GraphQLBuilder
  /Service.Tests

We are on the same page. @aaronburtle, could you please do these changes?

@aaronburtle
Copy link
Contributor Author

aaronburtle commented Feb 23, 2023

Should we flatten the src/Cli folder structure as part of this PR or in a separate one?

I think we can fit that into this PR as well, how should we flatten the src/Cli folder structure? You mean simply eliminating src and test within Cli, have it all just within Cli itself?

having both src files and test files within Cli itself would make it unorganized where both source files and test files are mixed up.
@aaronpowell, did you have a different proposal for flattening src/Cli?
how about we create a new folder for src/Cli.Test? src/Cli gets files that are currently under src/Cli/src src/Cli.Tests gets files which are today under src/Cli/test

My thought it what @aaronburtle suggests, as that would have the CLI folders fitting into the rest of the folder structure.
It'd see this:

/src
  /Auth
  /Cli
  /Cli.Tests
  /Config
  /Service
  /Service.GraphQLBuilder
  /Service.Tests

We are on the same page. @aaronburtle, could you please do these changes?

Sounds good, adding to this PR.

Aniruddh25
Aniruddh25 reviewed Feb 25, 2023
View reviewed changes
Aniruddh25
Aniruddh25 reviewed Feb 25, 2023
View reviewed changes
Aniruddh25
Aniruddh25 approved these changes Feb 28, 2023
View reviewed changes
Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

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

Thanks for incorporating additional fixes for flattening the Cli folder structure

@Aniruddh25 Aniruddh25 merged commit aa2ca4a into main Feb 28, 2023
@Aniruddh25 Aniruddh25 deleted the dev/aaronburtle/fixFolderStructure branch February 28, 2023 02:09
@Aniruddh25 Aniruddh25 mentioned this pull request Mar 3, 2023
Merged
Aniruddh25 added a commit that referenced this pull request Mar 3, 2023
@Aniruddh25
Fix expected path for built binaries for code signing (#1302)
c545a4e 
## Why make this change?

- The PR #1241 fixed our folder structure of the output binaries and in the process of making cli and engine paths consistent removed the `buildConfiguration` from the output path. 
- The code signing task still depended the binaries to be in the previous path. We didn't catch this before since a new release was not attempted earlier to recognize we may hit this issue. 

## What is this change?

- Modify the path where the code signing task looks for the binaries.
ayush3797 pushed a commit that referenced this pull request Mar 14, 2023
@Aniruddh25 @ayush3797
Fix expected path for built binaries for code signing (#1302)
cf2d867 
## Why make this change?

- The PR #1241 fixed our folder structure of the output binaries and in the process of making cli and engine paths consistent removed the `buildConfiguration` from the output path. 
- The code signing task still depended the binaries to be in the previous path. We didn't catch this before since a new release was not attempted earlier to recognize we may hit this issue. 

## What is this change?

- Modify the path where the code signing task looks for the binaries.
@ayush3797 ayush3797 mentioned this pull request Mar 14, 2023
Merged
Aniruddh25 added a commit that referenced this pull request Mar 15, 2023
@ayush3797 @Aniruddh25
Fix expected path for built binaries for code signing (#1302) (#1326)
aedc066 
## Why make this change?

- The PR #1241 fixed our folder structure of the output binaries and in the process of making cli and engine paths consistent removed the `buildConfiguration` from the output path. 
- The code signing task still depended the binaries to be in the previous path. We didn't catch this before since a new release was not attempted earlier to recognize we may hit this issue. 

## What is this change?

- Modify the path where the code signing task looks for the binaries.

Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
abhishekkumams pushed a commit that referenced this pull request Mar 30, 2023
@ayush3797 @Aniruddh25
Fix expected path for built binaries for code signing (#1302) (#1326)
fa133fc 
## Why make this change?

- The PR #1241 fixed our folder structure of the output binaries and in the process of making cli and engine paths consistent removed the `buildConfiguration` from the output path. 
- The code signing task still depended the binaries to be in the previous path. We didn't catch this before since a new release was not attempted earlier to recognize we may hit this issue. 

## What is this change?

- Modify the path where the code signing task looks for the binaries.

Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Aniruddh25 Aniruddh25 Aniruddh25 approved these changes

@seantleonard seantleonard seantleonard approved these changes

@mbhaskar mbhaskar Awaiting requested review from mbhaskar

@JelteF JelteF Awaiting requested review from JelteF

@Mathos1432 Mathos1432 Awaiting requested review from Mathos1432

@gledis69 gledis69 Awaiting requested review from gledis69

@milismsft milismsft Awaiting requested review from milismsft

@junsu0ms junsu0ms Awaiting requested review from junsu0ms

@tarazou9 tarazou9 Awaiting requested review from tarazou9

@ayush3797 ayush3797 Awaiting requested review from ayush3797

@abhishekkumams abhishekkumams Awaiting requested review from abhishekkumams

@aaronpowell aaronpowell Awaiting requested review from aaronpowell

@severussundar severussundar Awaiting requested review from severussundar

@ravishetye ravishetye Awaiting requested review from ravishetye

Assignees

@aaronburtle aaronburtle

Labels

cleanup

Projects

Data API builder
Status: Done

Milestone

Open Sourcing Milestone

Development

Successfully merging this pull request may close these issues.

Fix folder structure

5 participants

@aaronburtle @seantleonard @Aniruddh25 @aaronpowell @abhishekkumams