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

Replaced Items’ String-based names with proper Paths #233

Merged
merged 1 commit into from
Oct 24, 2018

Conversation

eqrion
Copy link
Collaborator

@eqrion eqrion commented Oct 22, 2018

This is a rebased and updated version of #201.

@eqrion
Copy link
Collaborator Author

eqrion commented Oct 22, 2018

@regexident Could you give this a glance? It's your work rebased and updated.

It looks good to me, but I'd like another pair of eyes to make sure I didn't mess up anything your were doing.

@eqrion
Copy link
Collaborator Author

eqrion commented Oct 22, 2018

There is one functional change here that I can see. We should now mangle the names of the tag enum for C headers when we generate monomorphs. As far as I can tell, this was a bug that we weren't doing this.

@regexident
Copy link
Contributor

@regexident Could you give this a glance? It's your work rebased and updated.

Looks good to me. Thanks for the rebase!

As far as I can tell, this was a bug that we weren't doing this.

Yes, I stumbled upon it as well.

@regexident
Copy link
Contributor

I think the GitHub outage bricked the CI queue. You may have to manually re-trigger Travis-CI.

@eqrion eqrion force-pushed the fix/path-all-the-things branch from 8ecaa05 to ffff824 Compare October 24, 2018 16:26
@eqrion
Copy link
Collaborator Author

eqrion commented Oct 24, 2018

Looks like the test failed because of rustfmt. Here's another try.

@eqrion eqrion merged commit 26828c9 into master Oct 24, 2018
@eqrion
Copy link
Collaborator Author

eqrion commented Oct 24, 2018

Fixed and merged. Thanks @regexident!

@regexident
Copy link
Contributor

@emilio
Copy link
Collaborator

emilio commented Oct 30, 2018

This change should probably have been a breaking change. It made the include config option not work on the renamed path, but in the original one, which broke mozilla-central.

I'll update cbindgen on mozilla-central, so no action needed I guess, but wanted to point that out.

@emilio
Copy link
Collaborator

emilio commented Oct 30, 2018

Hmm, actually I think it may be better if we keep it the way it was... I'll see how hard it is to fix.

@emilio
Copy link
Collaborator

emilio commented Oct 30, 2018

Actually I think the new behavior is saner, let's keep that, add a test, and I'll take care of all versions building with old cbindgen versions.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 31, 2018
mozilla/cbindgen#233 changed the way one of the options
we use work.

I think the new behavior is better, but we should do this sooner rather than
later, and fix broken builds.

Differential Revision: https://phabricator.services.mozilla.com/D10301

--HG--
extra : moz-landing-system : lando
emilio added a commit to emilio/servo that referenced this pull request Nov 5, 2018
mozilla/cbindgen#233 changed the way one of the options
we use work.

I think the new behavior is better, but we should do this sooner rather than
later, and fix broken builds.

Differential Revision: https://phabricator.services.mozilla.com/D10301
emilio added a commit to emilio/servo that referenced this pull request Nov 5, 2018
mozilla/cbindgen#233 changed the way one of the options
we use work.

I think the new behavior is better, but we should do this sooner rather than
later, and fix broken builds.

Differential Revision: https://phabricator.services.mozilla.com/D10301
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
mozilla/cbindgen#233 changed the way one of the options
we use work.

I think the new behavior is better, but we should do this sooner rather than
later, and fix broken builds.

Differential Revision: https://phabricator.services.mozilla.com/D10301

UltraBlame original commit: 5b0e4820c7fbd5c21451c9e72b103b5555664e3a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
mozilla/cbindgen#233 changed the way one of the options
we use work.

I think the new behavior is better, but we should do this sooner rather than
later, and fix broken builds.

Differential Revision: https://phabricator.services.mozilla.com/D10301

UltraBlame original commit: 5b0e4820c7fbd5c21451c9e72b103b5555664e3a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 3, 2019
mozilla/cbindgen#233 changed the way one of the options
we use work.

I think the new behavior is better, but we should do this sooner rather than
later, and fix broken builds.

Differential Revision: https://phabricator.services.mozilla.com/D10301

UltraBlame original commit: 5b0e4820c7fbd5c21451c9e72b103b5555664e3a
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.

3 participants