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

x/sys: extract Unix includes into separate files #23412

Closed
Androbin opened this Issue Jan 11, 2018 · 10 comments

Comments

Projects
None yet
7 participants
@Androbin

Androbin commented Jan 11, 2018

I have already submitted a PR for this on the GitHub mirror:
golang/sys#6

Unfortunately, I was unable to re-submit it via Gerrit.
Maybe someone else bothers to make the changes?

@ALTree ALTree changed the title from Extract Unix includes into separate files to x/sys: extract Unix includes into separate files Jan 11, 2018

@gopherbot gopherbot added this to the Unreleased milestone Jan 11, 2018

@ALTree

This comment has been minimized.

Show comment
Hide comment
@ALTree

ALTree Jan 11, 2018

Member

Was this proposed change discussed anywhere? Is there a reference issue? Or you are proposing the change now, here, and it needs discussion?

Member

ALTree commented Jan 11, 2018

Was this proposed change discussed anywhere? Is there a reference issue? Or you are proposing the change now, here, and it needs discussion?

@Androbin

This comment has been minimized.

Show comment
Hide comment
@Androbin

Androbin Jan 11, 2018

There is no other ticket or discussion associated with the PR changeset.

Androbin commented Jan 11, 2018

There is no other ticket or discussion associated with the PR changeset.

@ALTree

This comment has been minimized.

Show comment
Hide comment
@ALTree
Member

ALTree commented Jan 11, 2018

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Jan 11, 2018

Member

Sorry, we can't look at patches that are sent as PRs, without CLAs on file. Gerrit enforces CLAs.

Please describe the problem in English and we can decide whether somebody can fix the problem independently.

Or submit it via Gerrit.

Or wait for us to start accepting GitHub PRs, soonish. (with the Google CLA bot)

Member

bradfitz commented Jan 11, 2018

Sorry, we can't look at patches that are sent as PRs, without CLAs on file. Gerrit enforces CLAs.

Please describe the problem in English and we can decide whether somebody can fix the problem independently.

Or submit it via Gerrit.

Or wait for us to start accepting GitHub PRs, soonish. (with the Google CLA bot)

@Androbin

This comment has been minimized.

Show comment
Hide comment
@Androbin

Androbin Jan 11, 2018

@bradfitz I have signed the CLA a while ago (via the Google CLA bot) and Gerrit lists it under "Agreements".

Androbin commented Jan 11, 2018

@bradfitz I have signed the CLA a while ago (via the Google CLA bot) and Gerrit lists it under "Agreements".

@ianlancetaylor

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor

ianlancetaylor Jan 11, 2018

Contributor

@Androbin What is the problem? Please tell us the problem first, then the solution. Thanks.

Contributor

ianlancetaylor commented Jan 11, 2018

@Androbin What is the problem? Please tell us the problem first, then the solution. Thanks.

@Androbin

This comment has been minimized.

Show comment
Hide comment
@Androbin

Androbin Jan 11, 2018

The script unix/mkerrors.sh contains both

  • inlined C code, mostly - but not limited to - includes
  • Shell code, generating Go code

I have extracted the former into separate files.
The refactoring is supposed to separate concerns.
This also makes both of these easier to change.

@ianlancetaylor I have described this in the firstmost comment of the PR.

Androbin commented Jan 11, 2018

The script unix/mkerrors.sh contains both

  • inlined C code, mostly - but not limited to - includes
  • Shell code, generating Go code

I have extracted the former into separate files.
The refactoring is supposed to separate concerns.
This also makes both of these easier to change.

@ianlancetaylor I have described this in the firstmost comment of the PR.

@tklauser

This comment has been minimized.

Show comment
Hide comment
@tklauser

tklauser Jan 12, 2018

Member

The script unix/mkerrors.sh contains both

  • inlined C code, mostly - but not limited to - includes
  • Shell code, generating Go code

I don't quite see a problem with that. FWIW I find it convenient to have the C code inlined in mkerrors.sh as the changes usually consist of adding a header and adjusting the regex in the same file. Splitting them out would mean having to remember to edit one (or even several) more files for each added/changed constant.

Member

tklauser commented Jan 12, 2018

The script unix/mkerrors.sh contains both

  • inlined C code, mostly - but not limited to - includes
  • Shell code, generating Go code

I don't quite see a problem with that. FWIW I find it convenient to have the C code inlined in mkerrors.sh as the changes usually consist of adding a header and adjusting the regex in the same file. Splitting them out would mean having to remember to edit one (or even several) more files for each added/changed constant.

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Mar 26, 2018

Contributor

This doesn't seem to be solving a real problem. Why do you want to do this?

Contributor

rsc commented Mar 26, 2018

This doesn't seem to be solving a real problem. Why do you want to do this?

@Androbin

This comment has been minimized.

Show comment
Hide comment
@Androbin

Androbin Mar 26, 2018

As I said, I thought it would be more convenient to have them separated.
Since this doesn't seem to be the case here, I'm abandoning this suggestion.

Androbin commented Mar 26, 2018

As I said, I thought it would be more convenient to have them separated.
Since this doesn't seem to be the case here, I'm abandoning this suggestion.

@Androbin Androbin closed this Mar 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment