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

ocaml tool to extract opcode information #803

Merged
merged 2 commits into from Dec 29, 2016

Conversation

Projects
None yet
3 participants
@nojb
Contributor

nojb commented Sep 11, 2016

Re-submission of #449, rebased and squashed onto trunk.

@mshinwell

This comment has been minimized.

Show comment
Hide comment
@mshinwell

mshinwell Dec 9, 2016

Contributor

@nojb Please resolve conflicts
@gasche Please confirm you still approve, like you said for #449.

Contributor

mshinwell commented Dec 9, 2016

@nojb Please resolve conflicts
@gasche Please confirm you still approve, like you said for #449.

@nojb

This comment has been minimized.

Show comment
Hide comment
@nojb

nojb Dec 9, 2016

Contributor

Done.

Contributor

nojb commented Dec 9, 2016

Done.

@gasche

This comment has been minimized.

Show comment
Hide comment
@gasche

gasche Dec 9, 2016

Member

I approve and @damiendoligez in #449 seemed happy with the patch as well.

On first sight this seems to fall under Xavier's "if it ain't broke..." principle, but the idea comes from Xavier in the first place and we have had much more trouble with sed than with ocaml in the build system in the past, so it does seem wise.

Member

gasche commented Dec 9, 2016

I approve and @damiendoligez in #449 seemed happy with the patch as well.

On first sight this seems to fall under Xavier's "if it ain't broke..." principle, but the idea comes from Xavier in the first place and we have had much more trouble with sed than with ocaml in the build system in the past, so it does seem wise.

@gasche gasche added the approved label Dec 9, 2016

@mshinwell

This comment has been minimized.

Show comment
Hide comment
@mshinwell

mshinwell Dec 27, 2016

Contributor

@nojb Please add a Changes entry.

Contributor

mshinwell commented Dec 27, 2016

@nojb Please add a Changes entry.

@mshinwell mshinwell added this to the 4.05.0 milestone Dec 27, 2016

@nojb

This comment has been minimized.

Show comment
Hide comment
@nojb

nojb Dec 28, 2016

Contributor

I thought only user-visible changes go into Changes.

Contributor

nojb commented Dec 28, 2016

I thought only user-visible changes go into Changes.

@gasche

This comment has been minimized.

Show comment
Hide comment
@gasche

gasche Dec 28, 2016

Member

It's also a way to keep a trace of the people that make nice contributions. The "### Compiler distribution build system" category would be a good fit for this change.

Member

gasche commented Dec 28, 2016

It's also a way to keep a trace of the people that make nice contributions. The "### Compiler distribution build system" category would be a good fit for this change.

nojb added some commits Sep 11, 2016

@nojb

This comment has been minimized.

Show comment
Hide comment
@nojb

nojb Dec 29, 2016

Contributor

Done. Thanks!

Contributor

nojb commented Dec 29, 2016

Done. Thanks!

@gasche gasche merged commit 8d8e6e9 into ocaml:trunk Dec 29, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gasche

This comment has been minimized.

Show comment
Hide comment
@gasche

gasche Dec 29, 2016

Member

Thanks, merged.

Member

gasche commented Dec 29, 2016

Thanks, merged.

@nojb nojb deleted the nojb:make_opcodes_in_ml branch Feb 24, 2017

camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017

Merge pull request #803 from nojb/make_opcodes_in_ml
ocaml tool to extract opcode information
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment