Suggestions (Moving forward)

[wentjun]

Hi there! Spoke to one of your team members earlier on Facebook Messenger, and I took a quick look at the codebase.

As this project grows, I have a couple of suggestions to make it scalable, on top of what was raised during our conversation.

1) Architecture.

• Right now, we have a lot of different types of files in each directory, and I believe there is a better way to structure this. For instance, media files (such as icons) can be stored on a directory called assets, and each component can be stored within its own directory based on the function, or even the route. In addition, pure UI components should be moved to its own directory.
• What I have suggested here may be subjective, as there is no 1 solution when it comes to architecture, hence we should keep an open discussion when it comes to this.

2) Git structure

• Instead of directly pushing to master, a more definite architecture would be to create a separate develop branch for development, whereby feature branches can be branched off from it. Keep the master branch clean for deployment.

3) CSS methodologies.

• Instead of having inline styles, I would recommend the team to isolate styles to CSS modules, or usage of CSS-in-JS (such as styled-components).

4) PWAs.

• Setting of up service workers and manifest.json.

5) Linting.

• Setting up of ESLint (for JS) and CSS linters (for instance, StyleLint)

Happy to keep this discussion open. In fact, I am happy to work on any of the tickets, though my time may be limited as I can only contribute during my free time.

[achrinza]

media files (such as icons) can be stored on a directory called assets

+1. This can help keep the project well-organised. Currently, everything is coupled together under src and a separation of assets from components would be a good starting point.

I would recommend the team to isolate styles to CSS modules, or usage of CSS-in-JS (such as styled-components).

+1 for using CSS modules. This would keep the project modular and easier to maintain.

Instead of directly pushing to master, a more definite architecture would be to create a separate develop branch for developmen

This is a subjective opinion, but I would recommend tagging working releases of the application instead of creating a long-lived develop branch.

A well-covered (as in test coverage) will ensure that the application is in a usable state before merging into master. #25 is the first step to implementing this, though the tests still need to be written and coverage needs to be checked (e.g. with Coveralls).

Furthermore, this will simplify the Git workflow. For example, if the develop branch is in an unusable state and we need to push a hotfix, how would that process look like? We can create a new hotfix branch and merge into master directly, but that may result in merge conflicts.

Quite a number of new projects merge directly into master and use version tagging and a robust test suite to ensure a "working state" is available.

Overall, I don't see the benefit of a develop branch over the newer convention. It's unfamiliar to newcomers and I think we should try keep the barrier to contribute as low as possible.

Setting up of ESLint (for JS)

25 implements this

[wentjun]

Thanks @achrinza! Absolutely agree with your points. Another plus point of CSS modules is that is plays with with SCSS (I think someone mentioned about adopting it in one of the earlier issues). LIkewise, I am open to using any one of the JSS libraries out there. I don't have a clear preference as both methodologies have their pros and cons.

And great work on setting up the CI pipeline.

[limyifan1]

Thanks everyone :) will work to merge what PRed now! Still rather new to this so had to do some googling to understand haha