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

libpgm 5.3.128 #95457

Closed
wants to merge 5 commits into from
Closed

libpgm 5.3.128 #95457

wants to merge 5 commits into from

Conversation

branchvincent
Copy link
Member

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

#93940

This needs a test

@BrewTestBot BrewTestBot added missing license Formula has a missing license which should be added no Linux bottle Formula has no Linux bottle labels Feb 19, 2022
@branchvincent branchvincent added the CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. label Feb 19, 2022
@danielnachun
Copy link
Member

It looks like this package contains x86 assembly code, which is unsurprising given that it is designed to interface at a low level with hardware. So I think this needs depends_on arch: :x86_64 as it is non-trivial for this formula to natively support ARM64.

@danielnachun
Copy link
Member

For the test I'd recommend taking some C code from the testing folder here: https://github.com/steve-o/openpgm/tree/master/openpgm/pgm/test, remove all the ifdef sections for WIN32, and then have it compile to a very small application that will include the libpgm headers, and dynamically link to the libpgm shared libraries. This type of test is usually sufficient to catch major issues with the package, without requiring too much code.

Copy link
Member

@SMillerDev SMillerDev left a comment

Choose a reason for hiding this comment

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

Should have a test before merging

@branchvincent
Copy link
Member Author

branchvincent commented Mar 9, 2022

Fumbled my way to a test, let me know if anything looks off

Formula/libpgm.rb Outdated Show resolved Hide resolved
@BrewTestBot BrewTestBot removed the missing license Formula has a missing license which should be added label Mar 11, 2022
Copy link
Member

@SMillerDev SMillerDev left a comment

Choose a reason for hiding this comment

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

Thanks @branchvincent ! Without your contributions it'd be impossible to keep homebrew going. You can feel good knowing that you've made the world a lot better for homebrew users around the world! 👍 🎉

@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

@branchvincent branchvincent deleted the libpgm branch March 11, 2022 14:45
@github-actions github-actions bot added the outdated PR was locked due to age label Apr 11, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. no Linux bottle Formula has no Linux bottle outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants