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

Lint #382

Closed
wants to merge 9 commits into from
Closed

Lint #382

wants to merge 9 commits into from

Conversation

Code0x58
Copy link
Contributor

@Code0x58 Code0x58 commented Aug 4, 2021

  • remove linting issues
  • require future PRs to not introduce any linting issues
  • fix tests on Windows which were denied access when trying to write to a file opened for only reading
  • remove testRename as an external call isn't necessary

edit: crudely fixing the lint issues shows a lot of returned errors which is making this PR grow. The lint golangci-lint job doesn't show everything at once so can be a game of whack-a-mole. Changing PR to draft until everything is sorted out

@Code0x58 Code0x58 force-pushed the lint branch 2 times, most recently from 52ab82b to 797dd7e Compare August 4, 2021 21:58
@Code0x58 Code0x58 marked this pull request as draft August 4, 2021 21:59
@Code0x58 Code0x58 force-pushed the lint branch 18 times, most recently from f53eaa0 to 8813d35 Compare August 7, 2021 01:54
@Code0x58 Code0x58 force-pushed the lint branch 2 times, most recently from f4731b2 to 23f11ea Compare August 7, 2021 19:07
@Code0x58 Code0x58 force-pushed the lint branch 2 times, most recently from 72be145 to 2f63702 Compare August 7, 2021 20:32
fsnotify.go Outdated Show resolved Hide resolved
fsnotify.go Outdated Show resolved Hide resolved
fsnotify.go Outdated Show resolved Hide resolved
fsnotify.go Outdated Show resolved Hide resolved
fsnotify.go Outdated Show resolved Hide resolved
fsnotify.go Outdated Show resolved Hide resolved
fsnotify.go Outdated Show resolved Hide resolved
@Code0x58 Code0x58 force-pushed the lint branch 8 times, most recently from a15b0b0 to a0f954f Compare August 8, 2021 11:02
…ying methods

Remove "bad file descriptor" race condition in inotify Close()
implementation where the background worker shuts down resources before
poller.wake() can use the resources
@nathany
Copy link
Contributor

nathany commented Aug 18, 2021

@Code0x58 How's it going on this? Should we configure golangci-lint to disable some of the lint checks for now -- to tackle some but not all right now?

Should we try to get this in for 1.5.0 #380?

@Code0x58
Copy link
Contributor Author

Code0x58 commented Aug 18, 2021

This was a bit of a can of worms, it revealed a few issues that led me to end up rewriting a fair bit. It'll need another evening or two of inspiration to get through it, but maybe a shorter term pass would be to ignore the messages as a nicety to new committers.

With the extent of changes, and lack of cost of different versions, I don't think it's worth hanging about for 1.5.0 - I'd be happy to see each merge result in a release (more so when an individual merge would be a minor).

@kolyshkin
Copy link
Contributor

@Code0x58 I have created #430 which intersects with this one a little. If you still have time and desire to work on this one, can you please rebase it on top of #430?

@arp242
Copy link
Member

arp242 commented Jul 21, 2022

This PR is kind of, ehm, large 😅 A number of changes are not obviously correct (changes in locking, errors, some semantics) and it's a lot to review and test. It would be best to split it in multiple PRs if you want to continue work on this.

@arp242
Copy link
Member

arp242 commented Jul 30, 2022

The lint errors/warnings should be fixed in the main branch now.

@arp242 arp242 closed this Jul 30, 2022
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.

None yet

5 participants