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

Update publishing docs #430

Merged
merged 1 commit into from
Jan 15, 2018
Merged

Update publishing docs #430

merged 1 commit into from
Jan 15, 2018

Conversation

hannalaakso
Copy link
Member

This PR updates the publish docs with more information, especially around managing NPM test user's access to new/deprecated packages.

Note: I am not convinced we need to include the steps for verifying changes by linking the package styles to the Design System (step 3) because we need an altogether better process of managing this step.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-430 January 12, 2018 12:54 Inactive
Copy link
Contributor

@NickColley NickColley left a comment

Choose a reason for hiding this comment

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

These changes look very good, I noticed you removed a step that included opening a PR halfway is it OK to do all this without doing a PR?

Copy link
Member

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

This is ace Hanna, thanks! One suggestion and one potential point of confusion that I think needs addressing.


3. For any new components, add a `package.json` inside `packages/component-name`:
3. Ensure all changes to `src/components/component-name` have been copied to `packages/component-name`. In order to verify this, do the following in the Design System:
Copy link
Member

Choose a reason for hiding this comment

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

I think this is worth doing, but doing this only verifies that the compiled packages work - it doesn't mean that all of the changes have been copied.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I've reworded this to make it clearer.

@@ -6,53 +6,49 @@ Lerna is a tool that optimizes the workflow around managing multi-package reposi

It manages dependencies between our components so that if the version number of `globals` is bumped, all components dependent on `globals` will also have their version bumped.

### Publishing components manually (while in Alpha)
### Publishing components manually (while in Alpha/Private beta)

In a new branch:
Copy link
Member

Choose a reason for hiding this comment

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

Do we want something about making sure you're up to date with the latest changes to master?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added.

```

6. Open a pull request for these changes.
Here, leave the version number blank to allow Lerna to complete it as part of `lerna publish`. Include any dependencies, such as the `icons` package, that the new component requires. Use the current version number of any packages that you include here, Lerna will update the version number as part of `lerna publish`.
Copy link
Member

Choose a reason for hiding this comment

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

👌

![Confirm publishing of changes in Lerna](./img/lerna-confirm-publish.png)

Then in a new branch:
11. If your changes included new packages, you need to grant the test user access to them:
Copy link
Member

Choose a reason for hiding this comment

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

Ace. 👌

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-430 January 12, 2018 14:18 Inactive
@36degrees
Copy link
Member

👏

@hannalaakso
Copy link
Member Author

@NickColley Yes I confirmed with @igloosi that a PR halfway through is unnecessary.

I'm leaving the PR open as I'd like @igloosi to cast his eye over the changes as well.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-430 January 12, 2018 14:33 Inactive
@kr8n3r
Copy link

kr8n3r commented Jan 15, 2018

it's fine. not sure about having to push tags. last release i did lerna publish pushed all git tags to remote and a commit with all the package.json changes
though i saw that hanna had issues in the last release

@hannalaakso hannalaakso merged commit 9fa1173 into master Jan 15, 2018
@hannalaakso hannalaakso deleted the update-publish-docs branch January 15, 2018 10:39
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.

None yet

5 participants