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

Check Spelling annotations not collapsing (OSOE-525) #175

Closed
Piedone opened this issue Jan 5, 2023 · 9 comments
Closed

Check Spelling annotations not collapsing (OSOE-525) #175

Piedone opened this issue Jan 5, 2023 · 9 comments
Labels
bug Something isn't working

Comments

@Piedone
Copy link
Member

Piedone commented Jan 5, 2023

Lately, the PR annotations of Check Spelling are not collapsing after the spelling errors are fixed, see e.g. here. These should rather look like this after the errors have been resolved (note "This comment has been minimized.", I opened it):

image

I suspect this is because of the longer advice configured. If indeed, then we should rather have a short advice.md that links to a longer documentation page.

Jira issue

@Piedone Piedone added the bug Something isn't working label Jan 5, 2023
@github-actions github-actions bot changed the title Check Spelling annotations not collapsing Check Spelling annotations not collapsing (OSOE-525) Jan 5, 2023
@BenedekFarkas
Copy link
Member

Same thing with the short version that links the one in this repo. Maybe the logic here is that having an advice.md means you want to keep it shown all the time or IDK.

@Piedone
Copy link
Member Author

Piedone commented Jan 5, 2023

What are you referring to, exactly? The screenshot I've include above uses advice.md but a shorter version, and it properly got collapsed.

@BenedekFarkas
Copy link
Member

BenedekFarkas commented Jan 5, 2023

That one! In DotNest Core Sites SDK for example (and at least one more repo I worked on this week), the last failure report wasn't collapsed after an error-free commit. I thought it's just GH having hiccups (it's been behaving strangely in the past week or so), so I collapsed it manually.

@Piedone
Copy link
Member Author

Piedone commented Jan 5, 2023

In a private repo, you've shown this:

image

The difference is that it refers to the comment being hidden as opposed to minimized. I've checked out older PRs and what seems to be the one that Check Spelling does is the "minimized" one (was the "hidden" one hidden manually?). So, as shown from this, indeed simply the shorter advice doesn't solve this (but having an advice is not a problem, the screenshot in the issue description shows that).

@Piedone
Copy link
Member Author

Piedone commented Jan 5, 2023

Hmm, this comment is properly collapsed (with a long advice, with "minimized") and was added at 11:48 PM CET on 4 January. This comment under the same PR isn't, and it was created at 3:37 PM CET on 5 January. Between these only this commit happened but that shouldn't cause it. Also, this uncollapsed comment was added on 3 January, so it's not related to some changes we made.

@Piedone
Copy link
Member Author

Piedone commented Jan 5, 2023

Opened a bug report: check-spelling/check-spelling#44

@BenedekFarkas
Copy link
Member

That's right, minimized is the automatic, hidden is manual one.

@Piedone
Copy link
Member Author

Piedone commented Jan 6, 2023

See from check-spelling/check-spelling#44 (comment). We need to change our implementation according to this.

@Piedone
Copy link
Member Author

Piedone commented Feb 21, 2023

Fixed in #205.

@Piedone Piedone closed this as completed Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants