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

Add changesetBaseRef configuration option for versioning (#1195) #1201

Merged
merged 4 commits into from
Apr 16, 2020

Conversation

fggrimshaw
Copy link
Contributor

What's the problem this PR addresses?

This PR implements issue #1195

How did you fix it?

  • Add changesetBaseRef, allowing for comparing files against non-master refs

  • Update yarnrc documentation accordingly

* Add changesetBaseRef, allowing for comparing files against non-master refs

* Update yarnrc documentation accordingly
export async function fetchBase(root: PortablePath) {
const candidateBases = [`master`, `origin/master`, `upstream/master`];
export async function fetchBase(root: PortablePath, {baseRef}: {baseRef?: string | null}) {
const candidateBases = baseRef !== null ? [baseRef] : [`master`, `origin/master`, `upstream/master`];
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking, what do you think about making it an array of refs instead, that would default to ['master', 'origin/master', 'upstream/master'] (what it currently is)? This way people who want to use a custom branch would be able to keep the behaviour that tries to take the origin into account if possible.

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 wanted to do that, but did not know how to use an array type. I see now I just need to add isArray: true. I will add this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey Maël, I have updated the code and documentation accordingly. I've tested it and it works well. The only limitation is that using the environment variable will only allow a single ref to be set, not a huge deal imo. That could also be user error, I didn't take the time to see if there was a syntax to pass an array.

fgrimshaw-tk and others added 3 commits April 15, 2020 11:26
* renamed changesetBaseRef to changesetBaseRefs

* updated its type to array, set master, origin/master, upstream/master as default

* remove hardcoded candidates, enforce non-nullable configuration

* update docs accordingly
* Add a usageError for when the changesetBaseRefs is explicitly set to the empty array

* ref -> refs in documentation
@arcanis arcanis merged commit 8e47cac into yarnpkg:master Apr 16, 2020
@arcanis
Copy link
Member

arcanis commented Apr 16, 2020

Thanks!

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.

3 participants