Details
Diff Detail
- Repository
- rP FreeBSD ports repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
You need to add a SUBDIR += eq10q-lv2 line to audio/Makefile
| audio/eq10q-lv2/Makefile | ||
|---|---|---|
| 1 ↗ | (On Diff #34492) | ^ These comments are being phased out [afaik], so I would not add them anylonger. |
| 5 ↗ | (On Diff #34492) | ^ DISTVERSION is prefered over PORTVERSION |
| 21 ↗ | (On Diff #34492) | ^ I would use cmake:outsource if it's possible |
| 35 ↗ | (On Diff #34492) | ^ using patches in files/ is often more readable than sed-calls. |
| audio/eq10q-lv2/Makefile | ||
|---|---|---|
| 32 ↗ | (On Diff #34492) | If this is for 10, it should be done with an OSVERSION check, so that it can be removed when the version of FreeBSD is not supported any more. |
| audio/eq10q-lv2/Makefile | ||
|---|---|---|
| 10 ↗ | (On Diff #34492) | @FreeBSD.org |
| audio/eq10q-lv2/Makefile | ||
|---|---|---|
| 21 ↗ | (On Diff #34492) | Only <30% of cmake ports use cmake:outsource. Normally, cmake projects build fine in-place. cmake:outsource is for situations when in-place build breaks. It usually breaks due to bugs. I think, USES=cmake is a good default. |
| 32 ↗ | (On Diff #34492) | This option fails in 10, but it is also not needed in general. This option is only meaningful in conjunction with the 'inline' keyword, that is deemed obsolete by clang (gcc might still make use of it). I will clarify the comment. |
| 35 ↗ | (On Diff #34492) | Recently another committer, ultima@, convinced me that the opposite is true for one-liner comments. Also, section 4.4.3 says "Simple replacements can be performed directly from the port Makefile using the in-place mode of sed". All these are one-line changes in each instance. |
| audio/eq10q-lv2/Makefile | ||
|---|---|---|
| 35 ↗ | (On Diff #34492) | It's down to taste, but in a patch-foo you always at least have some context around the change, which you don't have in the sed-calls. |
| audio/eq10q-lv2/Makefile | ||
|---|---|---|
| 21 ↗ | (On Diff #34492) | Well, not really, only <30% of ports use cmake:oursource because someone has not gone and replaced the rest with cmake:outsource, it should probably be the default, please, change it to cmake:outsource. |
| 35 ↗ | (On Diff #34492) | I agree with @tcberner here, use patches when possible (so, when you are not replacing by something variable), otherwise, you end up having a dozen sed one-liners that you have no idea if they are still:
With a patch, you know right away if it is still needed or not. |
Tobias, it seems that everything has been resolved, and it achieved the state of uncontroversialness. :)
Looks good.
| audio/eq10q-lv2/Makefile | ||
|---|---|---|
| 22 ↗ | (On Diff #35173) | ^ as a warning for the future. dos2unix eats everything - even binaries |
| audio/eq10q-lv2/Makefile | ||
|---|---|---|
| 22 ↗ | (On Diff #35173) | Ok. |