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

Move freetype binding to separate module. #221

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

waywardmonkeys
Copy link
Collaborator

This makes it more like the others, and will keep it cleaner when we go and bind the other functions in this module.

Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine to me and I like the cleanup. The only issue here is that the new file is missing a license header. I understand that there are many files missing headers, but that's another issue we will need to fix. :)

@waywardmonkeys
Copy link
Collaborator Author

Can I just add them all at once in something separate?

@mrobinson
Copy link
Member

Can I just add them all at once in something separate?

I think adding the license header now and cleaning up the rest of the files later makes more sense. It's not great to have code without a proper license checked in.

@waywardmonkeys
Copy link
Collaborator Author

Give me the text you want, and I'll add it.

None of the files in the harfbuzz-sys have a copyright / license header. The files in harfbuzz do. But the licenses are not the same. The harfbuzz-sys crate is MIT, while harfbuzz is MIT OR Apache 2.0.

@waywardmonkeys
Copy link
Collaborator Author

(That's why I don't want to do it here: It is likely to be bikeshedded and no files in this crate have one.)

@mrobinson
Copy link
Member

Give me the text you want, and I'll add it.

// Copyright 2023 The Servo Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

The header refers to both licenses. I assume that MIT is included here because that is what HarfBuzz uses, but the rust code itself is specifically licensed under Apache as well.

This makes it more like the others, and will keep it cleaner when
we go and bind the other functions in this module.

A license header is also added to match what is being added to
the rest of the files in another PR / commit.
@waywardmonkeys
Copy link
Collaborator Author

Okay, added here, and the rest of the files and such updated in #225.

Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mrobinson mrobinson added this pull request to the merge queue Aug 22, 2023
Merged via the queue into servo:master with commit ff7eb73 Aug 22, 2023
8 checks passed
@waywardmonkeys waywardmonkeys deleted the move-freetype-to-module branch August 22, 2023 16:23
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.

2 participants