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 CONTRIBUTING.md #3411

Merged
merged 4 commits into from
Feb 2, 2022
Merged

Update CONTRIBUTING.md #3411

merged 4 commits into from
Feb 2, 2022

Conversation

mifi
Copy link
Contributor

@mifi mifi commented Jan 5, 2022

No description provided.

Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

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

I think we should figure out how to fix yarn start rather than updating only these commands to npm in the docs while having everything else yarn.

@mifi
Copy link
Contributor Author

mifi commented Jan 5, 2022

Sure, I agree! Then this PR can serve as a TODO for fixing that

@Murderlon
Copy link
Member

I think we can close this. The title doesn't reveal the todo here while skimming through the PRs every now and then. I think @aduh95 was hoping #3363 may coincidentally fix the issue.

@mifi
Copy link
Contributor Author

mifi commented Jan 5, 2022

Sure. What about the orher change in this pr? You agree to that?

@Murderlon
Copy link
Member

Yes, that's good!

```

Releases are managed by GitHub Actions, here’s an overview of the process to release a new Uppy version:

* Run `yarn release` on your local machine.
* Follow the instructions and select what packages to release.
* Before committing, check if the generated files look good.
* When asked to edit the next CHANGELOG, only include changes related to the package(s) you selected for release.
Copy link
Member

Choose a reason for hiding this comment

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

Something worth noting is that if we skip some packages during a release, all the changes on those packages are "lost", meaning my tool won't know there are unreleased changes in those packages (it fetches all changes since latest release from GitHub, so commits that landed before are considered already released). I'm not sure if there is a way to work around that – I've thought of adding commit hashes to the CHANGELOG, and make the tool parse CHANGELOG to figure out which commits are unreleased. but I don't think it would scale if the tool has to check for 9k commits each time we want to do a release.

So maybe we should instead encourage releasers to not skip packages, unless the changes are unrelated to the npm package.

Copy link
Member

Choose a reason for hiding this comment

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

You could encourage it but it's a valid use case to for instance just do a security release for companion and no other packages. Then you do need to remove all the things from the changelog that are from the other packages.

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 see, so we should always release every package that has any changes, and make sure that the main branch is always stable and ready for release?

Copy link
Member

Choose a reason for hiding this comment

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

It seems I read this wrong the first time. If that's the case I think a big warning should be added to the instructions because it's important

Copy link
Member

Choose a reason for hiding this comment

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

Should we say something about not skipping packages unless the changes are actually not touching anything related to the npm package?

Copy link
Contributor Author

@mifi mifi Feb 1, 2022

Choose a reason for hiding this comment

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

sure! i updated the comment. feel free to edit

@Murderlon
Copy link
Member

@mifi can you update and merge?

@mifi
Copy link
Contributor Author

mifi commented Feb 1, 2022

Ok, like this?

@mifi mifi requested a review from Murderlon February 1, 2022 16:00
.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
Co-authored-by: Merlijn Vos <merlijn@soverin.net>
@mifi mifi merged commit 142b95a into main Feb 2, 2022
@mifi mifi deleted the improve-contributing branch February 2, 2022 15:31
@github-actions github-actions bot mentioned this pull request Feb 14, 2022
@aduh95 aduh95 mentioned this pull request Feb 14, 2022
github-actions bot pushed a commit that referenced this pull request Feb 14, 2022
| Package                   | Version | Package                   | Version |
| ------------------------- | ------- | ------------------------- | ------- |
| @uppy/companion           |   3.2.0 | @uppy/provider-views      |   2.0.7 |
| @uppy/companion-client    |   2.0.5 | @uppy/thumbnail-generator |   2.1.0 |
| @uppy/core                |   2.1.5 | @uppy/robodog             |   2.3.0 |
| @uppy/dashboard           |   2.1.4 | uppy                      |   2.5.0 |
| @uppy/locales             |   2.0.6 |                           |         |

- @uppy/companion: add support for COMPANION_UNSPLASH_SECRET (Mikael Finstad / #3463)
- @uppy/unsplash: fix nested meta (Artur Paikin / #3485)
- meta: fix(docs): typo in property `thumbnailType` (Dan Schalow / #3472)
- @uppy/robodog: add audio, box, unsplash, screen-capture to Robodog (Artur Paikin / #3483)
- meta: consolidate ENV files and fix contributing guidelines (Antoine du Hamel / #3475)
- @uppy/companion-client,@uppy/companion,@uppy/provider-views,@uppy/robodog: Finishing touches on Companion dynamic Oauth (Renée Kooi / #2802)
- meta: Improve companion docs (Mikael Finstad / #3479)
- meta: Make E2E Great Again (Merlijn Vos / #3444)
- meta: Add PostCSS handling to Vite (Artur Paikin / #3467)
- meta: Update CONTRIBUTING.md (Mikael Finstad / #3411)
- @uppy/companion: fix broken thumbnails for box and dropbox (Mikael Finstad / #3460)
- website: fix `Uppy is not defined` error (Antoine du Hamel / #3461)
- @uppy/companion: Implement periodic ping functionality (Mikael Finstad / #3246)
- @uppy/companion: fix callback urls (Mikael Finstad / #3458)
- @uppy/core,@uppy/dashboard,@uppy/thumbnail-generator: Add dashboard and UIPlugin types (Merlijn Vos / #3426)
- @uppy/locales: Add "save" to fr_FR.js (Charly Billaud / #3395)
- @uppy/companion: Fix TypeError when invalid initialization vector (Julian Gruber / #3416)
- meta: Upgrade size-limit to 7.0.5 (Artur Paikin / #3445)
- @uppy/provider-views: Unsplash: UI improvements (Artur Paikin / #3438)
- @uppy/thumbnail-generator: exifr: remove legacy IE support (Artur Paikin / #3382)
- @uppy/companion: Default to HEAD requests when the Companion looks to get meta information about a URL (Zack Bloom / #3417)
- @uppy/dashboard: check if info array is empty (Artur Paikin / #3442)
- meta: dev: fix Vite custom plugin (Antoine du Hamel / #3437)
- website: add legacy bundle to CDN example (Antoine du Hamel / #3433)
- meta: remove unused lerna and npm files (Antoine du Hamel / #3436)
- meta: replace browserify with esbuild (Antoine du Hamel / #3363)
HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
| Package                   | Version | Package                   | Version |
| ------------------------- | ------- | ------------------------- | ------- |
| @uppy/companion           |   3.2.0 | @uppy/provider-views      |   2.0.7 |
| @uppy/companion-client    |   2.0.5 | @uppy/thumbnail-generator |   2.1.0 |
| @uppy/core                |   2.1.5 | @uppy/robodog             |   2.3.0 |
| @uppy/dashboard           |   2.1.4 | uppy                      |   2.5.0 |
| @uppy/locales             |   2.0.6 |                           |         |

- @uppy/companion: add support for COMPANION_UNSPLASH_SECRET (Mikael Finstad / transloadit#3463)
- @uppy/unsplash: fix nested meta (Artur Paikin / transloadit#3485)
- meta: fix(docs): typo in property `thumbnailType` (Dan Schalow / transloadit#3472)
- @uppy/robodog: add audio, box, unsplash, screen-capture to Robodog (Artur Paikin / transloadit#3483)
- meta: consolidate ENV files and fix contributing guidelines (Antoine du Hamel / transloadit#3475)
- @uppy/companion-client,@uppy/companion,@uppy/provider-views,@uppy/robodog: Finishing touches on Companion dynamic Oauth (Renée Kooi / transloadit#2802)
- meta: Improve companion docs (Mikael Finstad / transloadit#3479)
- meta: Make E2E Great Again (Merlijn Vos / transloadit#3444)
- meta: Add PostCSS handling to Vite (Artur Paikin / transloadit#3467)
- meta: Update CONTRIBUTING.md (Mikael Finstad / transloadit#3411)
- @uppy/companion: fix broken thumbnails for box and dropbox (Mikael Finstad / transloadit#3460)
- website: fix `Uppy is not defined` error (Antoine du Hamel / transloadit#3461)
- @uppy/companion: Implement periodic ping functionality (Mikael Finstad / transloadit#3246)
- @uppy/companion: fix callback urls (Mikael Finstad / transloadit#3458)
- @uppy/core,@uppy/dashboard,@uppy/thumbnail-generator: Add dashboard and UIPlugin types (Merlijn Vos / transloadit#3426)
- @uppy/locales: Add "save" to fr_FR.js (Charly Billaud / transloadit#3395)
- @uppy/companion: Fix TypeError when invalid initialization vector (Julian Gruber / transloadit#3416)
- meta: Upgrade size-limit to 7.0.5 (Artur Paikin / transloadit#3445)
- @uppy/provider-views: Unsplash: UI improvements (Artur Paikin / transloadit#3438)
- @uppy/thumbnail-generator: exifr: remove legacy IE support (Artur Paikin / transloadit#3382)
- @uppy/companion: Default to HEAD requests when the Companion looks to get meta information about a URL (Zack Bloom / transloadit#3417)
- @uppy/dashboard: check if info array is empty (Artur Paikin / transloadit#3442)
- meta: dev: fix Vite custom plugin (Antoine du Hamel / transloadit#3437)
- website: add legacy bundle to CDN example (Antoine du Hamel / transloadit#3433)
- meta: remove unused lerna and npm files (Antoine du Hamel / transloadit#3436)
- meta: replace browserify with esbuild (Antoine du Hamel / transloadit#3363)
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

3 participants