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

Redesign the subset feature #34663

Merged
merged 4 commits into from
Apr 8, 2020
Merged

Conversation

ViktorHofer
Copy link
Member

Fixes #34403

Part of yesterday's batched rollout: #33821. I'll send out a mail to the team for the updated workflow instructions.

@ghost
Copy link

ghost commented Apr 7, 2020

Tagging @ViktorHofer as an area owner

Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

Looks great to me, thank you!

@vcsjones
Copy link
Member

vcsjones commented Apr 7, 2020

I'll send out a mail to the team for the updated workflow instructions.

Will this affect contributors? If so, can some of the workflow changes be shared somewhere?

@safern
Copy link
Member

safern commented Apr 7, 2020

Will this affect contributors? If so, can some of the workflow changes be shared somewhere?

I think every rollout date is going to be reminded on the gitter channel with the changes for that rollout.

@ViktorHofer
Copy link
Member Author

Will this affect contributors? If so, can some of the workflow changes be shared somewhere?

Yes, I pinned #33821 to make sure that community members know about the planned changes as well which also list the impact and the new workflow. In addition to that after this is merged I'll notify the gitter channel. The docs will be updated as part of this PR as well. Anything you are missing here?

ViktorHofer added a commit to dotnet/source-indexer that referenced this pull request Apr 7, 2020
@vcsjones
Copy link
Member

vcsjones commented Apr 7, 2020

pinned #33821

Anything you are missing here?

I guess eyes. Apologies for missing that (or perhaps I saw it and forgot about it, that sounds like something I'd do, too).

@ViktorHofer
Copy link
Member Author

We are doing this the first time (batched rollout + announcements) so there's definitely room for improvement. FWIW I just posted a little announcement in the gitter channel.

@ViktorHofer
Copy link
Member Author

Merging as CI seems to be busted and I verified the changes with the existing legs.

@ViktorHofer ViktorHofer merged commit 8e0147e into dotnet:master Apr 8, 2020
@ViktorHofer ViktorHofer deleted the subsetref branch April 8, 2020 01:09
@tmds
Copy link
Member

tmds commented Apr 8, 2020

@ViktorHofer I think this is a nice improvement. 👍

It seems the -subset argument needs to be specified, though the examples in #34403 didn't have it, e.g.:

./build.sh libs+clr

Is this due to a parsing limitation in the script?

nit: the documentation mixes use of -subset and --subset. It would be nice to pick one and use it consistently.

@danmoseley
Copy link
Member

cc @safern for question above as @ViktorHofer is out rest of week.

@safern
Copy link
Member

safern commented Apr 8, 2020

Is this due to a parsing limitation in the script?

Right now it needs to be passed only for bash. I'm working on fixing that -- but yeah initially was because of a limitation in the script.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redesign the subset feature
7 participants