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.

Sign up for GitHub

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

Jump to bottom

openwisp-monitoring: fix Makefile for 0.2.0 #25187

Merged
merged 2 commits into from
Nov 11, 2024
Merged

openwisp-monitoring: fix Makefile for 0.2.0 #25187

merged 2 commits into from
Nov 11, 2024

Conversation

pandafy
Copy link
Contributor

@pandafy pandafy commented Oct 22, 2024

Maintainer: @nemesifier
Compile tested: ramips
Run tested: ramips

@pandafy pandafy force-pushed the fix-openwisp-monitoring-0.2.0 branch from f42bc34 to 1c120eb Compare October 22, 2024 18:55
nemesifier
nemesifier approved these changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

LGTM ✔️

nemesifier
nemesifier reviewed Oct 24, 2024
View reviewed changes
Copy link
Contributor

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

@feckert we fixed some issues and updated some outdated code here.

@nemesifier
Copy link
Contributor

Any maintainer could merge this please? @BKPepe

@BKPepe
Copy link
Member

BKPepe commented Oct 29, 2024

Sure, maintainer could merge this, but commit subject seems vague to me and there is missing commit description.

BKPepe
BKPepe requested changes Oct 29, 2024
View reviewed changes
admin/openwisp-monitoring/Makefile Outdated Show resolved Hide resolved
admin/openwisp-monitoring/Makefile Show resolved Hide resolved
admin/openwisp-monitoring/Config.in Show resolved Hide resolved
admin/openwisp-monitoring/Makefile Outdated Show resolved Hide resolved
@pandafy pandafy changed the title openwisp-monitoring: fix Makefile for 1.1.0 openwisp-monitoring: fix Makefile for 0.2.0 Oct 29, 2024
@pandafy pandafy force-pushed the fix-openwisp-monitoring-0.2.0 branch from 1c120eb to 4e36c2e Compare October 29, 2024 15:54
@pandafy pandafy requested a review from BKPepe October 30, 2024 10:16
nemesifier
nemesifier reviewed Nov 2, 2024
View reviewed changes
Copy link
Contributor

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

it's ready for review again @BKPepe

@feckert
Copy link
Member

feckert commented Nov 5, 2024

The two commits are not quite right.
They are not separate! The new option NETJSON_MONITORING_IWINFO should be added in a seperate commit but the variable is already used in the first commit!

You should also not use sed when evaluating the variable CONFIG_NETJSON_MONITORING_IWINFO.
It can also be easier. Please use the following syntax https://github.com/openwrt/packages/blob/master/net/gensio/Makefile#L198

@BKPepe
Copy link
Member

BKPepe commented Nov 6, 2024

Good catch here, @feckert! :)

@nemesifier
Copy link
Contributor

The two commits are not quite right. They are not separate! The new option NETJSON_MONITORING_IWINFO should be added in a seperate commit but the variable is already used in the first commit!

You should also not use sed when evaluating the variable CONFIG_NETJSON_MONITORING_IWINFO. It can also be easier. Please use the following syntax https://github.com/openwrt/packages/blob/master/net/gensio/Makefile#L198

Thanks, we're working on it.

@pandafy pandafy force-pushed the fix-openwisp-monitoring-0.2.0 branch 2 times, most recently from 1745290 to 7d43da2 Compare November 7, 2024 15:34
@pandafy
openwisp-monitoring: fix Makefile for 0.2.0 update
3a4691d 
Commit 5e69da4 upgraded openwisp-monitoring
to version 0.2.0 but missed necessary Makefile adjustments, causing the
package to break in OpenWrt feeds.

This patch updates the Makefile to ensure proper functionality of
openwisp-monitoring with the 0.2.0 release.

Signed-off-by: Gagan Deep <[email protected]>
@pandafy pandafy force-pushed the fix-openwisp-monitoring-0.2.0 branch 2 times, most recently from f86f57c to 11d5bc0 Compare November 7, 2024 15:58
@pandafy
Copy link
Contributor Author

pandafy commented Nov 7, 2024

@BKPepe @feckert I have made the requested changes on this PR.

@feckert
Copy link
Member

feckert commented Nov 8, 2024

I would also suggest to bump the PKG_RELEASE by one. Everything else looks good to me and is ready to merge.

@pandafy
openwisp-monitoring: allow not depending on rpcd-mod-iwinfo
e2dc8a0 
Add option to exclude rpcd-mod-iwinfo from dependency.

Signed-off-by: Gagan Deep <[email protected]>
@pandafy pandafy force-pushed the fix-openwisp-monitoring-0.2.0 branch from 11d5bc0 to e2dc8a0 Compare November 8, 2024 13:18
@feckert feckert merged commit ca503cc into openwrt:master Nov 11, 2024
13 checks passed
@feckert
Copy link
Member

feckert commented Nov 11, 2024

Thanks merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nemesifier nemesifier nemesifier approved these changes

@feckert feckert feckert left review comments

@BKPepe BKPepe Awaiting requested review from BKPepe

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pandafy @nemesifier @BKPepe @feckert