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

[CI-NO-BUILD] [build] Scrub the cleaners #1211

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

benyamin-codez
Copy link
Contributor

Refactoring:
a) Standardised cleaner routines
b) Introduced -quiet and -debug parameters when run from repo root
c) Moved from FOR loop to a SHIFT loop to more reliably support a wider variety of directory formats and characters
d) Created NetKVM\clean_inx.cmd to delete *.inx files in NetKVM
e) Updated NetKVM\clean_dvl_log.cmd to use standardised routines
f) Moved all other custom clean operations to build\clean.bat
g) Added missing cleaner to viocrypt
h) Added missing entries for viomem and viocrypt to list of directories to clean

Remove the following missing sub-directories from cleaning:
a) viomem\app
b) viorng\coinstaller

a) Standardised cleaner routines
b) Introduced -quiet and -debug parameters when run from repo root
c) Moved from FOR loop to a shift loop to more reliably support a
   wider variety of directory formats and characters
d) Created NetKVM/clean_inx.cmd to delete *.inx files in NetKVM
e) Updated NetKVM/clean_dvl_log.cmd to use standardised routines
f) Moved all other custom clean operations to build/clean.bat
g) Added missing cleaner to viocrypt
h) Added missing entries for viomem and viocrypt to list of
   directories to clean

Signed-off-by: benyamin-codez <[email protected]>
Remove the following missing sub-directories from cleaning:
a) viomem\app
b) viorng\coinstaller

Signed-off-by: benyamin-codez <[email protected]>
@benyamin-codez
Copy link
Contributor Author

benyamin-codez commented Dec 5, 2024

The following now just call build\clean.bat.
The cleaner in the root of each directory path will still clean the relevant directory.
As such, perhaps we should consider removing them where convenient?

NetKVM/CoInstaller/clean.bat
NetKVM/Mof/clean.bat
NetKVM/NotifyObject/clean.bat
NetKVM/ProtocolService/clean.bat
VirtIO/WDF/clean.bat
viogpu/viogpuap/clean.bat
viogpu/viogpudo/clean.bat
viogpu/viogpusc/clean.bat
vioserial/app/cleanAll.bat

EDIT:
@YanVugenfirer @vrozenfe @ybendito

Let me know if you want me to add a commit removing any of these superfluous cleaners.
If so, I'll add that and update this PR to ready for review.
If not, I will update this PR to ready for review forthwith.

@YanVugenfirer
Copy link
Collaborator

@benyamin-codez thanks for all the PRs!

I am out in for the office until Wednesday. I will take a look after I am back.

@kostyanf14 kostyanf14 changed the title [build] Scrub the cleaners [CI-NO-BUILD] [build] Scrub the cleaners Dec 9, 2024
@kostyanf14
Copy link
Member

Added CI-NO-BUILD as this PR touch only cleans files that are not used during the build process.

@YanVugenfirer
Copy link
Collaborator

The following now just call build\clean.bat. The cleaner in the root of each directory path will still clean the relevant directory. As such, perhaps we should consider removing them where convenient?

NetKVM/CoInstaller/clean.bat
NetKVM/Mof/clean.bat
NetKVM/NotifyObject/clean.bat
NetKVM/ProtocolService/clean.bat
VirtIO/WDF/clean.bat
viogpu/viogpuap/clean.bat
viogpu/viogpudo/clean.bat
viogpu/viogpusc/clean.bat
vioserial/app/cleanAll.bat

EDIT: @YanVugenfirer @vrozenfe @ybendito

Let me know if you want me to add a commit removing any of these superfluous cleaners. If so, I'll add that and update this PR to ready for review. If not, I will update this PR to ready for review forthwith.

I think that we should keep them. As those directories have VS projects, they can be compiled standalone.

Addendum to 52d23db and 3bc554e.

1. Added entry for *.dvl-win1*.xml
2. Added entry for *.legacy_dvl_result.txt

Signed-off-by: benyamin-codez <[email protected]>
@benyamin-codez
Copy link
Contributor Author

@YanVugenfirer @vrozenfe @ybendito

That last commit (933b732) added rmfiles entries for files created as part of changes proposed in PRs #1210 and #1212.

Whilst there are no dependencies, would you prefer that those PRs progress more before I switch this to ready for review?

@YanVugenfirer
Copy link
Collaborator

@benyamin-codez I suggest to switch

@benyamin-codez benyamin-codez marked this pull request as ready for review December 15, 2024 16:42
Addendum to 52d23db, 3bc554e and 933b732.

1. Removed packaging folder from root\clean.bat per PR virtio-win#1209.

Signed-off-by: benyamin-codez <[email protected]>
@benyamin-codez
Copy link
Contributor Author

@YanVugenfirer @vrozenfe @ybendito

I have removed the packaging folder from root\clean.bat per PR #1209 and the folder itself per PR #1224.

I have retained the discrete but generic cleaners for the sub-projects per Yan's advice above:

I think that we should keep them. As those directories have VS projects, they can be compiled standalone.

This one should be right to go now... 8^d

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

Successfully merging this pull request may close these issues.

3 participants