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 script that validates all files in the repo contain ASCII-only bytes and no UTF-8 BOM. #1199

Merged
merged 6 commits into from
Sep 4, 2020

Conversation

ahsonkhan
Copy link
Member

@ahsonkhan ahsonkhan commented Sep 2, 2020

Part of #1201

As a follow up to #1198 which fixes any non-ASCII related issues we have, I wanted to enable a CI-validating leg for this so that we stay green.

@ahsonkhan ahsonkhan added this to the [2020] September milestone Sep 2, 2020
@ahsonkhan ahsonkhan self-assigned this Sep 2, 2020
@ahsonkhan
Copy link
Member Author

FYI, @vhvb1989

@ahsonkhan
Copy link
Member Author

ahsonkhan commented Sep 2, 2020

Blocked on Azure/azure-sdk-tools#946 and #1198

@ahsonkhan
Copy link
Member Author

ahsonkhan commented Sep 2, 2020

Files found with non-ASCII characters: 8

6 are being fixed by #1198 (although there are a couple of duplicates)
1 is being fixed by Azure/azure-sdk-tools#946
1 is about the build directory

@ahsonkhan
Copy link
Member Author

ahsonkhan commented Sep 4, 2020

1 is about the build directory

You can invoke the tool using the following command: reportgenerator
Tool 'dotnet-reportgenerator-globaltool' (version '4.6.6') was successfully installed.
You can invoke the tool using the following command: reportgenerator
Tool 'dotnet-reportgenerator-globaltool' (version '4.6.6') was successfully installed.

This latest version of the tool has the BOM fix.

@vhvb1989
Copy link
Member

vhvb1989 commented Sep 4, 2020

For src files I understand we would get compilation warnings,
but for non-src files like eng files and build (cmake), what's the benefit?

@ahsonkhan
Copy link
Member Author

but for non-src files like eng files and build (cmake), what's the benefit?

Consistency (especially around the BOM) and I don't currently see a harm or reason to exclude them. If we find one in the future, we can skip those.

Copy link
Member

@vhvb1989 vhvb1989 left a comment

Choose a reason for hiding this comment

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

Suggested a way to simplify scrip by just using recursive grep instead of a loop

eng/pipelines/templates/jobs/archetype-sdk-client.yml Outdated Show resolved Hide resolved
eng/pipelines/templates/jobs/archetype-sdk-client.yml Outdated Show resolved Hide resolved
eng/pipelines/templates/jobs/archetype-sdk-client.yml Outdated Show resolved Hide resolved
@ahsonkhan ahsonkhan merged commit a478f9e into Azure:master Sep 4, 2020
@ahsonkhan ahsonkhan deleted the AddASCIIValidation branch September 4, 2020 22:02
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