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

Use ArgumentException.ThrowIfNullOrWhitespace #48185

Closed
danmoseley opened this issue May 10, 2023 · 7 comments · Fixed by #52897
Closed

Use ArgumentException.ThrowIfNullOrWhitespace #48185

danmoseley opened this issue May 10, 2023 · 7 comments · Fixed by #52897
Assignees
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions help candidate Indicates that the issues may be a good fit for community to help with. Requires work from eng. team

Comments

@danmoseley
Copy link
Member

danmoseley commented May 10, 2023

implemented in dotnet/runtime#86007 and should flow here in a few days.

similar effort for ThrowIfNull #43482

there have been several of these throw helpers added in the core libraries this release, maybe there's others that are relevant to us. perhaps someone would like to take a look?

@danmoseley danmoseley added help wanted Up for grabs. We would accept a PR to help resolve this issue area-runtime labels May 10, 2023
@danmoseley
Copy link
Member Author

@amcasey do we have an area like runtime has 'area-meta' for cross codebase changes?

@amcasey
Copy link
Member

amcasey commented May 11, 2023

We ended up concluding that was out of scope for our initial clean-up push since it required more coordination. My personal feeling is that that probably wouldn't be an "area-" since (I understand) those labels are applied automatically. I have seen issues tagged with more than one area. Alternatively, some cross-team pushes get prefix-less labels.

@danmoseley
Copy link
Member Author

that probably wouldn't be an "area-" since (I understand) those labels are applied automatically

I have no opinion but FYI, that isn't a problem. dotnet/runtime has area-meta and it's predicted (probably with lower accuracy). @jeffhandley how well do you think that works -- thoughts about pros/cons of multiple area labels (which is forbidden in dotnet/runtime)?

@jeffhandley
Copy link
Member

In dotnet/runtime, area-meta label works okay. It ends up being a "junk drawer" label to catch all things that span multiple areas, represent cross-area/cross-team epics, as well as epics and stories that span multiple repos.

Where its value surfaces is that it makes accountability clearer: No individual area owners are expected to track the area-meta issues; instead, the leads follow up on them. Getting those issues out of the way for typical area ownership responsibilities helped the team.

@wcontayon
Copy link
Contributor

wcontayon commented May 20, 2023

Hi @danmoseley can I take a look and submit a PR on that issue 🙂 ?

@danmoseley
Copy link
Member Author

@wcontayon Go for it

@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
@divyeshio
Copy link
Contributor

Hi, I would like to work on this

@mkArtakMSFT mkArtakMSFT added help candidate Indicates that the issues may be a good fit for community to help with. Requires work from eng. team and removed help wanted Up for grabs. We would accept a PR to help resolve this issue labels Oct 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions help candidate Indicates that the issues may be a good fit for community to help with. Requires work from eng. team
Projects
None yet
7 participants
@jeffhandley @danmoseley @amcasey @wcontayon @mkArtakMSFT @divyeshio and others