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

apply issue fixes and apply pull requests #69

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

mgood7123
Copy link

@mgood7123 mgood7123 commented Aug 29, 2021

pulls applied:

#66 - CMake: Fix pkgconfig generation.
#65 - don't install a libtool file with static library
#60 - Install cmake files under lib
#59 - Apply the same settings used for Linux also to k*BSD and GNU Hurd
#56 - Windows static build: COMPILE_DEFINITIONS: GRAPHITE2_STATIC
#54 - Fix the static build on Darwin
#45 - Correct relative library path

pulls not applied:

#53 as is unclear what exactly should be done

issues fixed:

#68 - set_property could not find CACHE variable CMAKE_BUILD_TYPE
#61 - fix to build with Intel compiler

issues not fixed:

#55 as it is unclear if the suggested patch/fix is stable

@bgermann
Copy link
Contributor

It would have been great if you had kept the authors' names in the commits but thanks anyway.

@mgood7123
Copy link
Author

It would have been great if you had kept the authors' names in the commits but thanks anyway.

added :)

@lantw44
Copy link

lantw44 commented Oct 11, 2021

It would be nice if you could include this change. FreeBSD uses amd64 instead of x86_64.

-    if (${CMAKE_SYSTEM_PROCESSOR} MATCHES "x86|i.86")
+    if (${CMAKE_SYSTEM_PROCESSOR} MATCHES "x86|i.86|amd64")

@a17r
Copy link

a17r commented Dec 28, 2021

[non-graphite-dev] git messages should describe what they do, not what number of pull request or issue they incorporate. [/non-graphite-dev]

It would have been great if you had kept the authors' names in the commits but thanks anyway.

added :)

I don't see that? If you didn't use cherry-pick to get the pull request commits, you can re-establish original authorship with --amend --author="name <email>"

Why are you duplicating existing PRs anyway? Do they need changes? If so, which did you make?

@mgood7123
Copy link
Author

[non-graphite-dev] git messages should describe what they do, not what number of pull request or issue they incorporate. [/non-graphite-dev]

It would have been great if you had kept the authors' names in the commits but thanks anyway.

added :)

I don't see that? If you didn't use cherry-pick to get the pull request commits, you can re-establish original authorship with --amend --author="name <email>"

Why are you duplicating existing PRs anyway? Do they need changes? If so, which did you make?

simply to merge all into a single PR :)

@mgood7123
Copy link
Author

feel free to clone my repo and rewrite the history as you like, directly referencing the issues/pulls that the commit fixes is easier than trying to come up with an accurate description of what exactly has been changed

@a17r
Copy link

a17r commented Jan 8, 2022

No. git log shall be meaningful locally without visiting github links. This is a terrible way to organise a repository.

@mgood7123
Copy link
Author

mgood7123 commented Jan 18, 2022

No. git log shall be meaningful locally without visiting github links. This is a terrible way to organise a repository.

suggest some then :)

someone can probably come up with something that first under 50 chars for each commit :)

i dont have much time to rename the commits to be more meaningful

i can give you write access to my repo so you can rewrite the commit history to be more meaningful if you want

@mgood7123
Copy link
Author

[non-graphite-dev] git messages should describe what they do, not what number of pull request or issue they incorporate. [/non-graphite-dev]

It would have been great if you had kept the authors' names in the commits but thanks anyway.

added :)

I don't see that? If you didn't use cherry-pick to get the pull request commits, you can re-establish original authorship with --amend --author="name <email>"

Why are you duplicating existing PRs anyway? Do they need changes? If so, which did you make?

also i think it is easier to just merge a PR that already has a ton of PR/issues merged/fixed in a single go rather than trying to merge and fix each one

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