Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses critical build issues encountered when compiling Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly addresses issues with passing complex compiler and linker flags to apxs by properly quoting them using -Wc and -Wl. The added support for DESTDIR in the install step is also a good improvement for packaging. I've found one issue with the implementation that will cause make install to fail, related to the use of an undefined MKDIR_P variable. Please see my comment for details on how to fix it.
Ensure more complex default CFLAGS, e.g. on Fedora and RHEL, are handled gracefully by apxs. While building RPM packages, installation happens to $DESTDIR, this needs to be passed to apxs, too. Co-Authored-By: William Storey <wstorey@maxmind.com>
2 participants
Ensure more complex default
CFLAGS, e.g. on Fedora and RHEL, are handled gracefully byapxs. While building RPM packages, installation happens to$DESTDIR, this needs to be passed toapxs, too.My test happened with current Fedora Rawhide, but also with meanwhile aged RHEL 8.
Current situation:
Situation with this pull request: