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

Chore/starting doc plateform #1

Merged
merged 13 commits into from
May 8, 2023
Merged

Conversation

franckgaudin
Copy link

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented May 4, 2023

⚠️ No Changeset found

Latest commit: 00f270f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@netlify
Copy link

netlify bot commented May 4, 2023

Deploy Preview for wl-untitled ready!

Name Link
🔨 Latest commit 00f270f
🔍 Latest deploy log https://app.netlify.com/sites/wl-untitled/deploys/64591204a3b014000999720f
😎 Deploy Preview https://deploy-preview-1--wl-untitled.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@patricklafrance
Copy link
Member

Our standard is to use PascalCase to name components files.

layout.tsx -> Layout.tsx

@patricklafrance
Copy link
Member

patricklafrance commented May 5, 2023

(To be compliant with our policies, this repository should be configured to get at least one reviewer approval before merging back into main)

@alexasselin008 listed our policies here: https://github.com/workleap

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
apps/docs/app/tokens.css Outdated Show resolved Hide resolved
apps/docs/app/tokens.css Outdated Show resolved Hide resolved
apps/docs/app/tokens.css Outdated Show resolved Hide resolved
apps/docs/components/Header/Header.tsx Outdated Show resolved Hide resolved
apps/docs/components/Header/Header.tsx Outdated Show resolved Hide resolved
apps/docs/components/Header/Header.tsx Outdated Show resolved Hide resolved
apps/docs/app/tokens.css Outdated Show resolved Hide resolved
@franckgaudin
Copy link
Author

Our standard is to use PascalCase to name components files.

layout.tsx -> Layout.tsx

For the structure of the app, I based myself on the conventions of Nextjs. If we apply this convention in the structure of next we will end up with page.tsx, Layout.tsx, etc... because to render the content it is absolutely necessary that page is in lower case.

For everything that concerns the components ./components/* the convention is respected.

@franckgaudin
Copy link
Author

(To be compliant with our policies, this repository should be configured to get at least one reviewer approval before merging back into main)

@alexasselin008 listed our policies here: https://github.com/workleap

The configurations are in place, once the project is public they will be applied.

@franckgaudin franckgaudin requested a review from fraincs May 5, 2023 14:57
@patricklafrance
Copy link
Member

patricklafrance commented May 5, 2023

(To be compliant with our policies, this repository should be configured to get at least one reviewer approval before merging back into main)
@alexasselin008 listed our policies here: https://github.com/workleap

The configurations are in place, once the project is public they will be applied.

Got it, is there any specific reason for the project to not being public yet?

For SOC2 compliance those policies must be in place through the whole development effort.

.npmrc Outdated Show resolved Hide resolved
fraincs
fraincs previously approved these changes May 5, 2023
Copy link
Contributor

@fraincs fraincs left a comment

Choose a reason for hiding this comment

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

Good job

@franckgaudin franckgaudin requested a review from ofrogon May 5, 2023 17:45
ofrogon
ofrogon previously approved these changes May 5, 2023
@franckgaudin franckgaudin dismissed stale reviews from ofrogon and fraincs via 2093c6b May 8, 2023 12:50
@franckgaudin franckgaudin requested review from fraincs and ofrogon May 8, 2023 13:00
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
apps/docs/app/globals.css Outdated Show resolved Hide resolved
@franckgaudin franckgaudin requested a review from fraincs May 8, 2023 14:27
@franckgaudin franckgaudin merged commit 17cff8b into main May 8, 2023
@franckgaudin franckgaudin deleted the chore/starting-doc-plateform branch May 8, 2023 17:48
@patricklafrance
Copy link
Member

patricklafrance commented May 8, 2023

It seems to have been merged with unresolved issues, is there a plan to address those?

@franckgaudin
Copy link
Author

It seemed to me that all the issues were answered, what problem was not addressed?

@patricklafrance
Copy link
Member

There are 12 issues marked as "Outdated" rather than "Resolved". It doesn't seems possible to link to specific comment.

@franckgaudin
Copy link
Author

They are indicated as "Outdated" however they have been taken into account and modified. The status is added when code is pushed on the branch, I don't know if there is a way to change it by resolut.
Maybe you know it?

@patricklafrance
Copy link
Member

Good question, not sure!

Yeah we now expect to use Github Discussions for those rather than issues. It's more adapted and offer a great separation between actual bugs and discussions / questions.

What's you take on using Github Discussion for questions / discussions? This is something I am looking to push for every repo. I am wondering thought if it's overkill for repos like this one.

According to Netlify docs, it seem that you should specify a packageManager field with the required version.

Did you try the packageManager field? If so, does it work?

@franckgaudin
Copy link
Author

 Did you try the packageManager field? If so, does it work?

I set it up and it worked well 👍🏻

What's you take on using Github Discussion for questions / discussions? This is something I am looking to push for every repo. I am wondering thought if it's overkill for repos like this one.

In fact I created this repo with the intention of including the Design System Components, that's why it's a Monorepo. In this context I think that the implementation of Discussion is a good thing.

@patricklafrance
Copy link
Member

Awesome, thanks for sharing :)

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

Successfully merging this pull request may close these issues.

4 participants