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

Deprecate _pad field on siginfo_t #1329

Merged
merged 3 commits into from
May 2, 2019
Merged

Deprecate _pad field on siginfo_t #1329

merged 3 commits into from
May 2, 2019

Conversation

alex
Copy link
Member

@alex alex commented May 1, 2019

As discussed in #1316

@rust-highfive
Copy link

r? @gnzlbg

(rust_highfive has picked a reviewer for you, use r? to override)

@alex
Copy link
Member Author

alex commented May 1, 2019

Hmm, what's teh appropriate way to wrap this long line?

@gnzlbg
Copy link
Contributor

gnzlbg commented May 1, 2019

You can just break line at since and note.

Could you add the methods to access the field here as well ? The note should tell people to report here with their use case, and that they can upgrade their code to use the methods instead (but the available methods might not be enough).

@alex
Copy link
Member Author

alex commented May 1, 2019

A method to access _pad you mean?

@gnzlbg
Copy link
Contributor

gnzlbg commented May 1, 2019

I mean a method like si_addr in the other PR.

The problem is, we are telling people that pad is deprecated, so when they see that message, they would be like "ok, I shouldn't use pad, what should I use instead?". That's a fair question. Ideally, we would have enough methods implemented already so that we can just tell people to use the type's methods to access the appropriate fields.

That might not be feasible to do right now, and not all methods might be needed, but at least we should be able to tell people that "if you need a method that's not available, look how si_addr is implemented, and please submit a PR with the method you need and we will release that quickly", to minimize friction.

Only after everybody is able to port their code can we make the method private, and then continue the refactor towards using an union when possible.

@gnzlbg
Copy link
Contributor

gnzlbg commented May 2, 2019

So this LGTM. @bors: r+

@bors
Copy link
Contributor

bors commented May 2, 2019

📌 Commit 9b43ade has been approved by gnzlbg

@bors
Copy link
Contributor

bors commented May 2, 2019

⌛ Testing commit 9b43ade with merge 11f30a8...

bors added a commit that referenced this pull request May 2, 2019
Deprecate _pad field on siginfo_t

As discussed in #1316
@bors
Copy link
Contributor

bors commented May 2, 2019

☀️ Test successful - checks-cirrus, checks-travis, status-appveyor
Approved by: gnzlbg
Pushing 11f30a8 to master...

@bors bors merged commit 9b43ade into rust-lang:master May 2, 2019
@alex alex deleted the patch-1 branch May 2, 2019 11:36
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.

4 participants