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

Misc Improvements #13

Open
7 of 11 tasks
s0 opened this issue Jan 16, 2022 · 6 comments
Open
7 of 11 tasks

Misc Improvements #13

s0 opened this issue Jan 16, 2022 · 6 comments

Comments

@s0
Copy link
Contributor

s0 commented Jan 16, 2022

After playing around with this code base for a couple of days, I've had a lot of fun and would love to become more involved! I've had a few ideas for how things could be improved, and happy to go into more detail for any of them, but wanted to open an issue to check that you're open to these kinds of changes before going ahead and doing any of them?

  • Add CI checks for type errors, linting, formatting etc... using GitHub Actions
  • Increase strictness of TypeScript configuration, and eventually enable strict: true
    (I've made a couple of mistakes (like this one), that would have been caught if TypeScript settings were a little stricter).
  • Replace all synchronous I/O operations with async / await
  • Start using JavaScript naming conventions for symbols (variables, classes, etc...)
  • Use io-ts for definition of types for configuration
  • Use io-ts for definition of types for message-passing between windows
  • Pull out all "magic constants" into the consts.ts file introduced in Multi-monitor support #11 (especially shared strings etc..., so they don't get out-of-sync with one another)
  • Add explicit public / private modifiers to all class methods / properties
  • Replace any binds for methods with lambda methods
  • Migrate default themes to TypeScript
  • Enable sourcemaps for better debugging in both main process and renderer preload.

I'll add more things to this list as I think of them!

@JezerM
Copy link
Owner

JezerM commented Jan 16, 2022

Looks nice~

However, I don't know if I like to use asynchronous I/O operations instead of sync ones... I think this change is unnecessary as almost every I/O operation is done before the windows are started; the exceptions are the battery and brightness features, which didn't seem to block the Renderer process.

About io-ts, I didn't know about it, so I will have to check it~

@s0 s0 mentioned this issue Jan 17, 2022
3 tasks
@JezerM
Copy link
Owner

JezerM commented Jan 30, 2022

I think Github Actions are covered now:

  • ESLint and Prettier check on Push and Pull Request
  • Fix code with eslint and prettier through a Pull Request (fired manually)
  • Build deb package and publish on tag release

@s0 s0 mentioned this issue Feb 11, 2022
@s0
Copy link
Contributor Author

s0 commented Feb 11, 2022

Sorry for not responding sooner, been a busy couple of weeks for me! Glad to see that you've made progress with the CI checks and increasing strictness too! I've opened a PR to make a few further improvements to the CI, including running TS compilation checks, and checking package-lock.json is up-to-date too!

@JezerM
Copy link
Owner

JezerM commented Mar 9, 2022

Added explicit member accessibility (public/private) in 81bfa93. Also, the sourceMap option in tsconfig.json is set to true, so "Enable sourcemaps for better debugging..." should be covered~

@JezerM
Copy link
Owner

JezerM commented Apr 15, 2022

Default themes were migrated to TypeScript~ Now, I'm looking to remove the _vendors packages by using npm packages instead, if possible.

@JezerM
Copy link
Owner

JezerM commented Aug 1, 2022

JavaScript naming convention was adopted in 87944b1. The exceptions to this are JavaScript API objects like lightdm, greeter_config, theme_utils and greeter_comm, as this would deprecate all existing themes and would force theme maintainers to change API access. Perhaps, in the future, the latter can be done.

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

No branches or pull requests

2 participants