microsoft / winget-dsc

MIT License
23 stars 14 forks source link

Markdownlint GitHub #122

Closed Gijsreyn closed 1 week ago

Gijsreyn commented 2 weeks ago

Microsoft Reviewers: Open in CodeFlow
Gijsreyn commented 2 weeks ago

@denelon

github-actions[bot] commented 2 weeks ago

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view, the :scroll:action log, or :memo: job summary for details.

Unrecognized words (1)

markdownlint

These words are not needed and should be removed Toolpackage

To accept these unrecognized words as correct and remove the previously acknowledged and now absent words, you could run the following commands ... in a clone of the [git@github.com:Gijsreyn/winget-dsc.git](https://github.com/Gijsreyn/winget-dsc.git) repository on the `markdownlint-github` branch ([:information_source: how do I use this?]( https://docs.check-spelling.dev/Accepting-Suggestions)): ``` sh curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.24/apply.pl' | perl - 'https://github.com/microsoft/winget-dsc/actions/runs/11716527910/attempts/1' ```
Pattern suggestions :scissors: (3) You could add these patterns to `.github/actions/spelling/patterns.txt`: ``` # Automatically suggested patterns # hit-count: 13 file-count: 5 # Compiler flags (?:^|[\t ,"'`=(])-[DPWXYLlf](?=[A-Z]{2,}|[A-Z][a-z]|[a-z]{2,}) # hit-count: 3 file-count: 1 # githubusercontent /[-a-z0-9]+\.githubusercontent\.com/[-a-zA-Z0-9?&=_\/.]* # hit-count: 2 file-count: 1 # GitHub SHAs (markdown) (?:\[`?[0-9a-f]+`?\]\(https:/|)/(?:www\.|)github\.com(?:/[^/\s"]+){2,}(?:/[^/\s")]+)(?:[0-9a-f]+(?:[-0-9a-zA-Z/#.]*|)\b|) ```
Notices (1) #### See the [:open_file_folder: files](https://github.com/microsoft/winget-dsc/pull/122/files/) view, the [:scroll:action log](https://github.com/microsoft/winget-dsc/actions/runs/11716527910/job/32634729935#step:4:1), or [:memo: job summary](https://github.com/microsoft/winget-dsc/actions/runs/11716527910/attempts/1#summary-32634729935) for details. [:information_source: Notices](https://docs.check-spelling.dev/Event-descriptions) | Count -|- [:information_source: candidate-pattern](https://docs.check-spelling.dev/Event-descriptions#candidate-pattern) | 5 See [:information_source: Event descriptions](https://docs.check-spelling.dev/Event-descriptions) for more information.
If the flagged items are :exploding_head: false positives If items relate to a ... * binary file (or some other file you wouldn't want to check at all). Please add a file path to the `excludes.txt` file matching the containing file. File paths are Perl 5 Regular Expressions - you can [test]( https://www.regexplanet.com/advanced/perl/) yours before committing to verify it will match your files. `^` refers to the file's path from the root of the repository, so `^README\.md$` would exclude [README.md]( ../tree/HEAD/README.md) (on whichever branch you're using). * well-formed pattern. If you can write a [pattern](https://github.com/check-spelling/check-spelling/wiki/Configuration-Examples:-patterns) that would match it, try adding it to the `patterns.txt` file. Patterns are Perl 5 Regular Expressions - you can [test]( https://www.regexplanet.com/advanced/perl/) yours before committing to verify it will match your lines. Note that patterns can't match multiline strings.
Gijsreyn commented 2 weeks ago

Can you help me understand how this action works? Is it like the spell checking where it fails if the linting doesn't match, or is it something that actually goes in and fixes the linting?

The same as the spell checker. I didn't explicitly set the continueOnError, because $false is the default behaviour.

denelon commented 1 week ago

I saw some of the "security" related items in one of the files. We want folks to submit security related concerns through the official Microsoft link mentioned in SECURITY.md.

Gijsreyn commented 1 week ago

I saw some of the "security" related items in one of the files. We want folks to submit security related concerns through the official Microsoft link mentioned in SECURITY.md.

I didn't fully get what you mean Demitrius. You mind elaborating a bit more what you sawa and what might needs to be changed?

denelon commented 1 week ago

I didn't fully get what you mean Demitrius. You mind elaborating a bit more what you sawa and what might needs to be changed?

I reviewed it, I think it's fine.

Gijsreyn commented 1 week ago

@denelon Thanks! @ryfu-msft If you're also happy, you mind pulling it in?

ryfu-msft commented 1 week ago

/azp run

azure-pipelines[bot] commented 1 week ago
Azure Pipelines successfully started running 1 pipeline(s).
Gijsreyn commented 1 week ago

Ryan, I don't know why that prerelease is failing, whereas other runs it is successful. No file has been touched on tests. Can we create a separate issue for it for investigation and pull this in?

ryfu-msft commented 1 week ago

/azp run

azure-pipelines[bot] commented 1 week ago
Azure Pipelines successfully started running 1 pipeline(s).
ryfu-msft commented 1 week ago

Ryan, I don't know why that prerelease is failing, whereas other runs it is successful. No file has been touched on tests. Can we create a separate issue for it for investigation and pull this in?

Lets try one more time and see if its a one off.

Gijsreyn commented 1 week ago

At least from my side: image

Gijsreyn commented 1 week ago

I see what is going wrong, but I don't know if it is a bug in the DSC resource, or something on the dotnet.exe NuGet side. Version 9.0.0 has just been released, and it's picking that one. Can we comment out the test and I'll take a look at it in another issue? We still have one test that test for prerelease, which is PowerShell.

EDIT: When I run dotnet tool install dotnet-ef --prerelease --global, I would assume it is going to install: 9.0.0-rc.2.24474.1, but it doesn't.

EDIT 2: Looking at the description, not really:

image

@ryfu-msft Would the PreRelease property then make sense, or just instruct users if they want to install a rc for example, just add the full version?

ryfu-msft commented 1 week ago

I see what is going wrong, but I don't know if it is a bug in the DSC resource, or something on the dotnet.exe NuGet side. Version 9.0.0 has just been released, and it's picking that one. Can we comment out the test and I'll take a look at it in another issue? We still have one test that test for prerelease, which is PowerShell.

EDIT: When I run dotnet tool install dotnet-ef --prerelease --global, I would assume it is going to install: 9.0.0-rc.2.24474.1, but it doesn't.

EDIT 2: Looking at the description, not really:

image

@ryfu-msft Would the PreRelease property then make sense, or just instruct users if they want to install a rc for example, just add the full version?

Seems like --prerelease only works if there is a prerelease version that exists later the latest stable release. I would instruct users to resort to using the full version instead of --prerelease since you never know when the next preview release will come out.

Regarding the tests, it would be better to explicitly point to an older preview version so that the tests don't break in the future when a new release comes out.

In an ideal world, we should not be using real packages from the nuget gallery but instead stand up our own test package source.

Gijsreyn commented 1 week ago

I fully agree with you here Ryan. I'll hit up a small commit in just a second to fix the test and the one for VSCode. If you want, I can remove the --prerelease later. Otherwise, I put it in the docs now. Thanks for checking.