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

Re-organise the project structure #261

Closed
GentlemanHal opened this issue Dec 20, 2018 · 7 comments
Closed

Re-organise the project structure #261

GentlemanHal opened this issue Dec 20, 2018 · 7 comments
Assignees
Labels
for developers Changes that make development easier
Milestone

Comments

@GentlemanHal
Copy link
Member

The project currently has a lot of top level folders and files, and not all of them are obvious unless you know the specific tool. I think it would be helpful to re-organise the project structure, especially for people new to the code base.

Some ideas

If you use create-react-app and then eject you end up with a config folder that contains the webpack config. We could also probably move a lot of the babel and linting config to the same folder.

Extract the ci_stub_server into its own project and publish it to npm. We could start by giving it a separate package.json.

Move the ci scripts into the .circleci folder.

Create a journey_test and move the cypress stuff under that, giving it a seperate package.json. This would have the added benefit of speeding up the CI pipeline as we wouldn't be installing all dependencies twice in different stages.

@GentlemanHal GentlemanHal added the for developers Changes that make development easier label Dec 20, 2018
@joejag
Copy link
Contributor

joejag commented Dec 20, 2018

Cool. Some thoughts.

Let's play the journey_test before extracting the ci_stub_server so we know what our requirements are for it.

I use VSCode for JS dev and we've currently got the project in a way that it cannot auto-detect the settings. If we refactor the layout then I'd suggest we check it works without modification in VSCode. I can test this.

We have an artisanally created linting settings as the ones at the time were unappealing. I'd recommend we adopt the 'standard' community preferences - which crucially does away with pointless semicolons. So we have something like this in .eslintrc.json:

{
    "extends": [
        "standard",
        "plugin:jest/recommended"
    ]
}

@GentlemanHal GentlemanHal added this to the v4.0.0 milestone Dec 20, 2018
@GentlemanHal GentlemanHal self-assigned this Dec 21, 2018
@GentlemanHal
Copy link
Member Author

I think I've organised things as well as I can without doing anything crazy.

I updated and renamed the ci_stub_server to have its own package.json, which means it should be easier to extract at a later date.

I moved the cypress stuff under the test folder as I felt it matched our structure better, but it might be more confusing for people used to cypress?

I use VSCode for JS dev and we've currently got the project in a way that it cannot auto-detect the settings. If we refactor the layout then I'd suggest we check it works without modification in VSCode. I can test this.

Hopefully I haven't done anything that should have broken this, but if I have let me know (or fix it).

We have an artisanally created linting settings as the ones at the time were unappealing. I'd recommend we adopt the 'standard' community preferences - which crucially does away with pointless semicolons. So we have something like this in .eslintrc.json

Not sure what you mean by this as we already use the recommended settings with some minor tweaks. Either way I've moved the configuration to package.json and am happy if it can be simplified more but I don't plan on making any further changes against this issue.

Are you happy to QA the changes and close @joejag?

@GentlemanHal
Copy link
Member Author

I've created issue #263 for further improving cctray_xml_feed_mock

@joejag
Copy link
Contributor

joejag commented Jan 3, 2019

Just had a quick check, it looks like we've migrated from eslint to stylelint instead? I've installed the stylelint plugin for VSCode but it's not doing much. Perhaps it's in conflict with eslint. I'll try a few more things are report back.

I'm thinking that the .eslintrc.json with the contents written before would be best for newbies

@GentlemanHal
Copy link
Member Author

GentlemanHal commented Jan 6, 2019

Just had a quick check, it looks like we've migrated from eslint to stylelint instead?

We use both, eslint for the Javascript and stylelint for the SCSS.

@GentlemanHal
Copy link
Member Author

I'm thinking that the .eslintrc.json with the contents written before would be best for newbies

I agree it's probably more common to use the .eslintrc file rather than have the config in the package.json, but the same is true for all the tools we use.

Which is currently babel, browserslist, eslintConfig, eslintIgnore and stylelint and having that many .rc files in the root was just messy. Some we could move to the config folder but this would also be non standard for newbies and others I couldn't figure out how to easily move (e.g. babel and browserlist).

I'm quite happy with them all being in package.json its a standard place all the tools look so we don't have to worry about trying to point them at the correct place and it makes it clear they are all for the UI.

@GentlemanHal
Copy link
Member Author

I'm going to close this for now, if anyone has an problems with the restructure we can re-open or add new issues.

@GentlemanHal GentlemanHal modified the milestones: v4.0.0, v3.2.0 Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for developers Changes that make development easier
Projects
None yet
Development

No branches or pull requests

2 participants