search

AstraExt / astra-monitor

Resource Monitor for GNOME shell
GNU General Public License v3.0
281 stars 14 forks source link

refactor: implement prettier for code formatting #104

Closed mcmxcdev closed 5 months ago

mcmxcdev commented 5 months ago

Changes

Motivation

I was looking into contributing to astra-monitor, but even the simplest file change caused lots of reformatting of code.

So I started with implementing prettier for code formatting which leads to a consistent look of each file.

Side note

I would really recommend to commit a lockfile, since only then you can guarantee that contributors will have the same versions installed as yourself. I highly recommend to pick pnpm.

ljuzig commented 5 months ago

I agree with you that a proper code formatting is needed, but this might come at a very critical moment. I'm currently working in a local branch on GPU dedicated top panel header which currently involves 30+ files changed and about 5k lines of code. I'm worried that merging this before I merge my branch will end up into a nightmare merge.

There are some solutions I can think of, like I could commit the branch I'm working on, which is currently very experimental and not complete, and you could adapt your pull request to that branch which will then be merged into the main when ready. Otherwise you could wait for it to be published and then adapt this pull request. Another solution is a pull request before running pnpm format, waiting for my merge, but it may make less sense. If you have other solutions in your mind or suggestions they are welcome.

mcmxcdev commented 5 months ago

I agree with you that a proper code formatting is needed, but this might come at a very critical moment. I'm currently working in a local branch on GPU dedicated top panel header which currently involves 30+ files changed and about 5k lines of code. I'm worried that merging this before I merge my branch will end up into a nightmare merge.

There are some solutions I can think of, like I could commit the branch I'm working on, which is currently very experimental and not complete, and you could adapt your pull request to that branch which will then be merged into the main when ready. Otherwise you could wait for it to be published and then adapt this pull request. Another solution is a pull request before running pnpm format, waiting for my merge, but it may make less sense. If you have other solutions in your mind or suggestions they are welcome.

It's always good to open draft PRs asap, because then they can also be seen by other contributors who then might hold off on big refactorings like this.

Personally, I am never worried about merge conflicts if you know how to handle them and in this case it should actually be really easy to solve: If this PR gets merged and you pull it into your local branch, in the case of merge conflicts you can just always accept "current changes" (your own) over the formatting changes and no code is lost. Then you run pnpm format over the codebase once and it should all be having your new code and be shiny!

ljuzig commented 5 months ago

I added a couple of additional configurations to prettier and momentarily added eslint on top of prettier. There are some situations where I feel prettier produces much less readable code and I will try to create some custom eslint rules to achieve what I want on those situations, otherwise I will eventually drop eslint altogether.

ljuzig commented 5 months ago

I think, anyway, a better contribution documentation is missing, I didn't expect actually much contribution until we get out of beta. I will work on after the beta. About committing the lockfile, I'm using NPM, I should commit my lock file but still pnpm users wouldn't really benefit from that.