Skip to content
This repository was archived by the owner on Feb 15, 2022. It is now read-only.

Consolidate Lib Directories #346

Merged
merged 3 commits into from
May 12, 2021
Merged

Consolidate Lib Directories #346

merged 3 commits into from
May 12, 2021

Conversation

rdavison
Copy link
Contributor

@rdavison rdavison commented May 12, 2021

Issue: ocaml/ocaml.org#141

I consolidated the four directories bindings, common, components, and layouts into the lib directory. I also updated the tailwind.config, gitignore, bsconfig, and pages/_app.js files to account for the consolidation.

@vercel
Copy link

vercel bot commented May 12, 2021

@rdavison is attempting to deploy a commit to the ocaml Team on Vercel.

To accomplish this, @rdavison needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

@rdavison rdavison requested a review from a user May 12, 2021 19:40
@ghost
Copy link

ghost commented May 12, 2021

I consolidated the four directories bindings, common, components, and layouts into the lib directory.

I would use src similar to rescript-lang.org and tailwindcss.com implementation. I see two potential advantages of using src:

Overall, I don't care too strongly one way or another, but @agarwal had some strong opinions on this topic generally.

@agarwal
Copy link
Member

agarwal commented May 12, 2021

We discussed and decided:

  • src/ is where all library modules go.
  • pages/ stays at the root level. That's better to avoid extra hierarchy and since it is NextJS's default.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked through changes and also scanned through the files for any additional references to directory names. This PR looks ready to merge, once latest feedback has been incorporated.

As a sanity check, run npx yarn build and check the exit code of that command. Also, start the test server quickly and browse a few pages as a quick sanity check.

@rdavison
Copy link
Contributor Author

I moved lib to src in ocaml/ocaml.org@7e59f83

@rdavison
Copy link
Contributor Author

As a sanity check, run npx yarn build and check the exit code of that command. Also, start the test server quickly and browse a few pages as a quick sanity check.

Ran the sanity checks. Everything looks good to me.

@rdavison rdavison merged commit 25d47bd into master May 12, 2021
@rdavison rdavison deleted the rnd1/consolidate-lib-dirs branch May 12, 2021 20:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants