Page MenuHomeFreeBSD

net-mgmt/netdata: upgrade to 1.18.1
Needs RevisionPublic

Authored by samm on Oct 15 2019, 11:06 PM.

Details

Summary
  • Upgrade to 1.18
  • Change github org name to netdata
  • Add FreeBSD specific patch for the megacli plugin (already accepted by upstream)
Test Plan

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

samm created this revision.Oct 15 2019, 11:06 PM
samm added a comment.Oct 15 2019, 11:08 PM

Hm, please wait, found that make makepatch broken one of the patches. Re-creating

samm updated this revision to Diff 63327.Oct 15 2019, 11:12 PM

Fix one of the patches

samm updated this revision to Diff 63329.Oct 15 2019, 11:30 PM

Fix error in the "MOVED" file

samm added a comment.Oct 15 2019, 11:31 PM
This comment was removed by samm.
samm updated this revision to Diff 63330.Oct 15 2019, 11:34 PM

revert changes

samm edited the test plan for this revision. (Show Details)Oct 15 2019, 11:37 PM
samm set the repository for this revision to rP FreeBSD ports repository.Oct 16 2019, 12:31 AM
samm updated this revision to Diff 63332.Oct 16 2019, 12:39 AM
krion requested changes to this revision.Oct 16 2019, 8:00 AM
krion added inline comments.
Makefile
22

Don't need it, it's ${PORTNAME} by default

This revision now requires changes to proceed.Oct 16 2019, 8:00 AM
samm updated this revision to Diff 63376.Oct 16 2019, 6:34 PM

Remove GH_ACCOUNT as it is now matching the app name

samm marked an inline comment as done.Oct 16 2019, 6:34 PM
krion accepted this revision.Oct 16 2019, 6:40 PM

If it builds on poudriere - approved

This revision is now accepted and ready to land.Oct 16 2019, 6:40 PM
samm updated this revision to Diff 63478.Oct 19 2019, 4:11 PM
samm retitled this revision from net-mgmt/netdata: upgrade to 1.18 to net-mgmt/netdata: upgrade to 1.18.1.
samm edited the test plan for this revision. (Show Details)
  • Upgrade to 1.18.1
  • Remove patches already accepted by upstream
  • Remove linefeed to make portlint happy
This revision now requires review to proceed.Oct 19 2019, 4:11 PM
krion accepted this revision.Oct 19 2019, 4:23 PM
This revision is now accepted and ready to land.Oct 19 2019, 4:23 PM

I'm not sure which one to merge as I also have a patch https://reviews.freebsd.org/D21969

They're pretty similar with a few differences
Removal of USE_LDCONFIG is missing (not needed)
Make use of LTO selectable rather than having configure script using autodetection
pkg-plist isn't sorted alphabetical
Mine doesn't have pkg-descr updated and keeps the GH_ACCOUNT which I can of change if needed

...and I almost forgot, then sample configuration is also substantially simplified and updated.

Hi,
I've merged all changes to my patch but feel free to commit either one.

samm added a comment.Mon, Oct 21, 8:02 PM

Hi,
I've merged all changes to my patch but feel free to commit either one.

Hi, my suggestion is to disable LTO option for a 2 reasons:

  1. It is automatically enabled if supported
  2. It breaks the build on 11.2

Wouldn't it be a better idea to use OSVERSION instead?

araujo requested changes to this revision.Wed, Oct 23, 8:55 AM

@samm Could you double check with @daniel.engberg.lists_pyret.net the changes that he is proposing and perhaps after that we can close the D21969 review?

A single place makes more sense, however @daniel.engberg.lists_pyret.net opened the review first, so technically he will receive the credits for the patch submission. I just want a single review for this port update.

Best Regards,

This revision now requires changes to proceed.Wed, Oct 23, 8:55 AM
files/patch-collectors_python.d.plugin_python.d.plugin.in
7

typo? python 3.6 --> python3.6

samm added a comment.Thu, Oct 24, 8:12 PM

@samm Could you double check with @daniel.engberg.lists_pyret.net the changes that he is proposing and perhaps after that we can close the D21969 review?
A single place makes more sense, however @daniel.engberg.lists_pyret.net opened the review first, so technically he will receive the credits for the patch submission. I just want a single review for this port update.
Best Regards,

Hi, i am perfectly fine to give him credits and merge the patches. Main problem is that there is no feedback from the maintainer. What should we do to get this done?

araujo added a comment.EditedFri, Oct 25, 1:48 AM
In D22045#483993, @samm wrote:

@samm Could you double check with @daniel.engberg.lists_pyret.net the changes that he is proposing and perhaps after that we can close the D21969 review?
A single place makes more sense, however @daniel.engberg.lists_pyret.net opened the review first, so technically he will receive the credits for the patch submission. I just want a single review for this port update.
Best Regards,

Hi, i am perfectly fine to give him credits and merge the patches. Main problem is that there is no feedback from the maintainer. What should we do to get this done?

At PORTS HANDBOOK there is a section maintainer where it explains the TIMEOUT[0], the maintainer timeout is 14 days(2 weeks), in this case, as soon as one of your mentors approve this REVIEW you can commit and add this in the commit log:

Approved by: <nick> (mentor), maintainer (timeout 14 days)

Best Regards,

[0] https://www.freebsd.org/doc/en/books/porters-handbook/book.html#makefile-maintainer

So please, proceed to merge these two reviews and incorporate the submitter suggestions.

Br,

samm added a comment.Fri, Nov 1, 7:24 AM

I am apologize for the delay, was very busy on recent GIMP release. I will take care of this today @daniel.engberg.lists_pyret.net

krion added a comment.Thu, Nov 7, 8:04 PM

@mmokhi ping, you wanted to go through this patch couple of weeks ago, something changed?

@samm Can you move forward with this patch? The maintainer has timeout already.

@krion @araujo
Hi,
Since this seems to have stalled, can you commit my patch found here (https://reviews.freebsd.org/D22045#485880) which addresses all known issues so we can get this out the door?

krion added a comment.EditedSat, Nov 16, 9:46 AM

mmokhi@ pinged me on twitter and told he's no access to internet at this moment and gave an approval for this patch, @samm could you commit your or @daniel.engberg.lists_pyret.net changes which are pretty similar anyway. Thanks.