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

Update README with release changes and contribution clarifications #1436

Merged
merged 5 commits into from
Dec 13, 2024
Merged

Update README with release changes and contribution clarifications #1436

merged 5 commits into from
Dec 13, 2024

Conversation

jamigibbs
Copy link
Contributor

@jamigibbs jamigibbs commented Dec 12, 2024 •
edited
Loading

Chromatic

https://readme-contributions-storybook--65a6e2ed2314f7b8f98609d8.chromatic.com

Description

While helping out a contributor from outside of DST, I noticed that there were a few things unclear and outdated in our Readme. Specifically around making contributions. Some things have changed too with creating a release so might as well tidy that up while I was there.

QA Checklist

  • Component maintains 1:1 parity with design mocks
  • Text is consistent with what's been provided in the mocks
  • Component behaves as expected across breakpoints
  • Accessibility expert has signed off on code changes (if applicable. If not applicable provide reason why)
  • Designer has signed off on changes (if applicable. If not applicable provide reason why)
  • Tab order and focus state work as expected
  • Changes have been tested against screen readers (if applicable. If not applicable provide reason why)
  • New components are covered by e2e tests; updates to existing components are covered by existing test suite
  • Changes have been tested in vets-website using Verdaccio (if applicable. If not applicable provide reason why)

Screenshots

Acceptance criteria

  • QA checklist has been completed
  • Screenshots have been attached that cover desktop and mobile screens

Definition of done

  • Documentation has been updated, if applicable
  • A link has been provided to the originating GitHub issue (or connected to it via ZenHub)

@jamigibbs jamigibbs added the ignore-for-release Used if you want to ignore the PR in the generated release notes label Dec 12, 2024
@jamigibbs jamigibbs changed the title adjust placement of sections and update deployment steps Update README with release changes and contribution clarifications Dec 12, 2024
jamigibbs
jamigibbs commented Dec 12, 2024
View reviewed changes
README.md

This will allow you to run Storybook locally to view all components

# Stencil Dev Server for Testing IE11
Copy link
Contributor Author

@jamigibbs jamigibbs Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm removing the Stencil Dev Server for Testing IE11 section because it seems pretty outdated. We don't support IE11 anymore so there shouldn't be a need to test there right? I've only ever done development with Storybook. Is there any reason we should keep these steps?

jamigibbs
jamigibbs commented Dec 12, 2024
View reviewed changes
README.md
@@ -198,95 +293,4 @@ This repo follows [semantic versioning](https://semver.org/). Here are some exam

- Accessibility fix
- Styling fix
- Functionality fix

### Releasing
Copy link
Contributor Author

@jamigibbs jamigibbs Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I moved the Releasing section up.

jamigibbs
jamigibbs commented Dec 12, 2024
View reviewed changes
README.md

## Testing

### Running tests for web components
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the Testing section up.

jamigibbs
jamigibbs commented Dec 12, 2024
View reviewed changes
README.md
@@ -179,6 +240,40 @@ To publish changes from the `web-components` subpackage, make sure the version n

[`yarn version`](https://yarnpkg.com/cli/version) is available to use to make changes in the CLI. To change the version of the package you are working on run one of the following commands: `yarn version major`, `yarn version minor`, or `yarn version patch`. For guidance on which command to use please see below.


### Releasing
Copy link
Contributor Author

@jamigibbs jamigibbs Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I made a few additions to the Release steps because since they were written, we have changed the release PR to increment more than just the core version number. And also a couple of clarifying things with what we do after the release is created (PRs for other repos, etc).

@jamigibbs jamigibbs marked this pull request as ready for review December 12, 2024 22:25
@jamigibbs jamigibbs requested a review from a team as a code owner December 12, 2024 22:25
@jamigibbs jamigibbs merged commit 4275f99 into main Dec 13, 2024
10 checks passed
@jamigibbs jamigibbs deleted the readme-contributions-storybook branch December 13, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@powellkerry powellkerry powellkerry approved these changes

Assignees
No one assigned
Labels
ignore-for-release Used if you want to ignore the PR in the generated release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jamigibbs @powellkerry