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

fix: make it os compatible. #24040

Merged
merged 3 commits into from
Feb 6, 2025
Merged

Conversation

Gofastasf
Copy link
Contributor

@Gofastasf Gofastasf commented Feb 1, 2025

Remove strings.Contains to:

  • Update findCoverageFile to match exact file names.
  • Enhance includePreviewPredicate for exact names.
  • Direct file checking of go.mod.
  • Remove hardcoded previewSubdir const

Remove path packge methods:

  • Use path/filepath methods to make it cross platform.
  • Use filepath.Ext for file extension.

path package methods are used for url and unix specific operation.
filepath pacakge methods are used for file system operation and cross platform.
Use filepath.Ext for file system extension.
Copy link

github-actions bot commented Feb 1, 2025

Thank you for your contribution @Gofastasf! We will review the pull request and get back to you soon.

@github-actions github-actions bot added Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. Internal Storage Storage Service (Queues, Blobs, Files) labels Feb 1, 2025
@Gofastasf
Copy link
Contributor Author

#24026

Copy link
Member

@jhendrixMSFT jhendrixMSFT left a comment

Choose a reason for hiding this comment

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

Changes to storage has introduced regressions like in #24039

- Update findCoverageFile to match exact file names.
- Enhance includePreviewPredicate for exact names.
- Direct file checking of go.mod.
- Removed hardcoded previewSubdir const.
@Gofastasf Gofastasf force-pushed the fix/replace-strings.Contains branch from 4e0941a to ac60429 Compare February 4, 2025 03:46
@Gofastasf
Copy link
Contributor Author

@jhendrixMSFT Hey resolved the CI issue. I guess its ready to get merged?

@jhendrixMSFT
Copy link
Member

Do we still need #24039?

@Gofastasf
Copy link
Contributor Author

Gofastasf commented Feb 4, 2025

@jhendrixMSFT Closed #24039.

@Gofastasf Gofastasf closed this Feb 4, 2025
@Gofastasf Gofastasf reopened this Feb 4, 2025
@Gofastasf Gofastasf changed the title Fix/replace strings.Contains fix: make it os compatible. Feb 4, 2025
@Gofastasf
Copy link
Contributor Author

@benbp Hey can you checkout this PR ? I think its ready to get merged!

@benbp benbp merged commit 5442c00 into Azure:main Feb 6, 2025
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. Internal Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants