-
-
Notifications
You must be signed in to change notification settings - Fork 62
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
⬆️ Update Grocy to v3.2.0 #284
Conversation
I just noticed that the fix_braindamage.patch patch fails due to this commit. I have adapted and tested the patch: diff --git a/views/layout/default.blade.php b/views/layout/default.blade.php
index 4eb3fb8d..d4e5e007 100644
--- a/views/layout/default.blade.php
+++ b/views/layout/default.blade.php
@@ -88,6 +88,10 @@
Grocy.Mode = '{{ GROCY_MODE }}';
Grocy.BaseUrl = '{{ $U('/') }}';
Grocy.CurrentUrlRelative = "/" + window.location.href.split('?')[0].replace(Grocy.BaseUrl, "");
+ if(Grocy.BaseUrl.indexOf('http') != 0 || Grocy.BaseUrl.indexOf('hassio_ingress') >= 0) {
+ Grocy.BaseUrl = window.location.origin + '{{ $U('/') }}';
+ Grocy.CurrentUrlRelative = "/" + window.location.pathname.replace(Grocy.BaseUrl, "");
+ };
Grocy.ActiveNav = '@yield('activeNav', '')';
Grocy.Currency = '{{ GROCY_CURRENCY }}';
Grocy.CalendarFirstDayOfWeek = '{{ GROCY_CALENDAR_FIRST_DAY_OF_WEEK }}'; |
ed3ffa6
to
fea1fe8
Compare
Is anything blocking this (besides @frenck's time)? |
@jlsjonas working on the add-on as we speak... |
This PR does contain multiple things that don't belong in a single PR. So it won't be merged though. |
Apart from the unfortunate commit message naming the diff feels quite in scope? (bumping grocy & fixing related items + patch-bumping used php8 modules as part of a general version bump feel quite related) But your call ;) not my PR :) |
@jlsjonas it isn't. It contains a bug fix, an Grocy upgrade, a dependency bump and support for a new feature. I appreciate one tries to help, but PR should never have multiple concerns squashed together. |
I just noticed that the [fix_braindamage.patch](https://github.com/hassio-addons/addon-grocy/blob/main/grocy/rootfs/patches/fix_braindamage.patch) patch fails due to [this](grocy/grocy@51fdefa) commit.
Agreed and misconception on my end, I thought the patch fix was due to the version bump. |
Sorry if I mixed things up. These were my reasons:
|
I'm happy to modify/split this PR if that would unblock it from getting merged. |
I've rebased the PR and pulled out the dependency upgrades that way. Let's go ahead and merge the rest in one. In general, it's nice for someone to read "Catalan" was added as a language (or other changes that are made). Those will be hidden in a single upgrade message (which is not transparent). |
(close/re-open was to re-trigger CI, GitHub was having some hickups earlier today) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @malcp 👍
Great! And fair point, I'll keep this in mind for next time. |
Proposed Changes
Bumped Grocy version to v3.2.0. The addon was tested following the "Local Add-on Testing" in https://developers.home-assistant.io/docs/add-ons/testing/
Related Issues