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.

Sign up for GitHub

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

Jump to bottom

Improvement/storybook docs #661

Merged
merged 51 commits into from
Jan 22, 2024
Merged

Improvement/storybook docs #661

merged 51 commits into from
Jan 22, 2024

Conversation

JeanMarcMilletScality
Copy link
Contributor

Component:

Description:

Design:

Breaking Changes:

  • [] Breaking Changes

Closes: #ISSUE_NUMBER

@bert-e
Copy link
Contributor

bert-e commented Dec 5, 2023

Hello jeanmarcmilletscality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

@bert-e
Copy link
Contributor

bert-e commented Dec 5, 2023

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

@bert-e
Copy link
Contributor

bert-e commented Dec 5, 2023

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

@ChengYanJin ChengYanJin requested a review from Cuervino December 5, 2023 15:57
ChengYanJin
ChengYanJin reviewed Dec 5, 2023
View reviewed changes
Copy link
Contributor

@ChengYanJin ChengYanJin left a comment

Choose a reason for hiding this comment

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

it's nice to have the guidance next to the component !
Just a few nitpicking to address.

src/lib/components/tooltip/Tooltip.component.tsx Outdated Show resolved Hide resolved
stories/Button/Guideline.mdx Outdated Show resolved Hide resolved
stories/Button/button.stories.tsx Outdated Show resolved Hide resolved
@bert-e
Copy link
Contributor

bert-e commented Dec 6, 2023

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

JBWatenbergScality
JBWatenbergScality reviewed Dec 6, 2023
View reviewed changes
stories/Button/Guideline.mdx Outdated Show resolved Hide resolved
stories/Button/Guideline.mdx Outdated Show resolved Hide resolved
stories/Button/Guideline.mdx Outdated Show resolved Hide resolved
stories/Button/Guideline.mdx Outdated Show resolved Hide resolved
stories/Button/Guideline.mdx Outdated Show resolved Hide resolved
stories/Select/Guideline.mdx Outdated Show resolved Hide resolved
stories/guideline/docs-template.mdx Outdated Show resolved Hide resolved
@bert-e
Copy link
Contributor

bert-e commented Dec 6, 2023

Conflict

There is a conflict between your branch improvement/storybook-docs and the
destination branch development/1.0.

Please resolve the conflict on the feature branch (improvement/storybook-docs).

git fetch && \
git checkout origin/improvement/storybook-docs && \
git merge origin/development/1.0

Resolve merge conflicts and commit

git push origin HEAD:improvement/storybook-docs

@bert-e
Copy link
Contributor

bert-e commented Dec 27, 2023

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

Cuervino
Cuervino reviewed Dec 27, 2023
View reviewed changes
Copy link
Contributor

@Cuervino Cuervino left a comment

Choose a reason for hiding this comment

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

I added few comments, that might be added to the text. Otherwise, let's go with this version.

stories/Button/Guideline.mdx Outdated Show resolved Hide resolved
stories/Button/Guideline.mdx Outdated Show resolved Hide resolved
stories/Button/Guideline.mdx Outdated Show resolved Hide resolved
@bert-e
Copy link
Contributor

bert-e commented Dec 28, 2023

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

@bert-e
Copy link
Contributor

bert-e commented Jan 9, 2024

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

JBWatenbergScality
JBWatenbergScality reviewed Jan 22, 2024
View reviewed changes
Copy link
Contributor

@JBWatenbergScality JBWatenbergScality left a comment

Choose a reason for hiding this comment

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

Overall LGTM, just a tiny comment on users pronoun, prefer usage of a generic inclusive pronoun ;) !

stories/InfoMessage/infomessage.guideline.mdx Outdated Show resolved Hide resolved
stories/Select/select.guideline.mdx

<Meta title="Guideline" of={SelectStories} />

# Select
Copy link
Contributor

Choose a reason for hiding this comment

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

@Cuervino maybe for later on we need to think about a loading state for select component requiring async process to get the options

@bert-e
Copy link
Contributor

bert-e commented Jan 22, 2024

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

@JeanMarcMilletScality @JBWatenbergScality
Update stories/InfoMessage/infomessage.guideline.mdx
260d7b7 
Co-authored-by: JBWatenbergScality <75977494+JBWatenbergScality@users.noreply.github.com>
JBWatenbergScality
JBWatenbergScality approved these changes Jan 22, 2024
View reviewed changes
Copy link
Contributor

@JBWatenbergScality JBWatenbergScality left a comment

Choose a reason for hiding this comment

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

LGTM !

@JeanMarcMilletScality
Copy link
Contributor Author

/approve

@bert-e
Copy link
Contributor

bert-e commented Jan 22, 2024

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

The following options are set: approve

@bert-e
Copy link
Contributor

bert-e commented Jan 22, 2024

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

  • ✔️ development/1.0

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

  • Any commit you add on the source branch will trigger a new cycle after the
    current queue is merged.
  • Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

@JBWatenbergScality
Copy link
Contributor

/status

@bert-e
Copy link
Contributor

bert-e commented Jan 22, 2024

Status

Status report is not available.

The following options are set: approve

@bert-e
Copy link
Contributor

bert-e commented Jan 22, 2024

I have successfully merged the changeset of this pull request
into targetted development branches:

  • ✔️ development/1.0

Please check the status of the associated issue None.

Goodbye jeanmarcmilletscality.

@bert-e bert-e merged commit 4e3236a into development/1.0 Jan 22, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChengYanJin ChengYanJin ChengYanJin left review comments

@JBWatenbergScality JBWatenbergScality JBWatenbergScality approved these changes

@Cuervino Cuervino Awaiting requested review from Cuervino

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JeanMarcMilletScality @bert-e @JBWatenbergScality @ChengYanJin @Cuervino