lunny/helm-chart
1
0
Fork 0
Code Issues 36 Pull Requests 14 Actions Packages Projects Releases 93 Wiki Activity

Update README with new dependency versioning approach #578

Merged
pat-s merged 5 commits from refs/pull/578/head into main 2023-12-13 16:56:02 +00:00
Conversation 1 Commits 5 Files Changed 1 +53 -5
pat-s commented 2023-12-04 20:23:38 +00:00 (Migrated from gitea.com)
Copy Link

As discussed in https://gitea.com/gitea/helm-chart/issues/572.

Also added a bit more context and updates to the overall "Dependencies" sections.

As discussed in https://gitea.com/gitea/helm-chart/issues/572. Also added a bit more context and updates to the overall "Dependencies" sections.
justusbunsi (Migrated from gitea.com) reviewed 2023-12-04 20:23:47 +00:00
pat-s commented 2023-12-11 07:59:30 +00:00 (Migrated from gitea.com)
Copy Link

@justusbunsi Any comments from your side?

@justusbunsi Any comments from your side?
justusbunsi (Migrated from gitea.com) reviewed 2023-12-11 17:22:44 +00:00
justusbunsi (Migrated from gitea.com) left a comment
Copy Link

Sorry for not reviewing earlier. Last week was weird wrt my availability.

Sorry for not reviewing earlier. Last week was weird wrt my availability.
README.md
justusbunsi (Migrated from gitea.com) commented 2023-12-11 17:24:17 +00:00
Copy Link

Don't repeat "dependencies".

- This chart provides those dependencies as sub-chart dependencies.
+ This chart provides those dependencies via sub-charts.
Don't repeat "dependencies". ```diff - This chart provides those dependencies as sub-chart dependencies. + This chart provides those dependencies via sub-charts. ```
README.md
justusbunsi (Migrated from gitea.com) commented 2023-12-11 17:27:00 +00:00
Copy Link

This should be more eye-catching. Maybe by using the quote syntax? Although, this would break the text flow. 🤔

This should be more eye-catching. Maybe by using the quote syntax? Although, this would break the text flow. 🤔
justusbunsi (Migrated from gitea.com) commented 2023-12-12 13:52:34 +00:00
Copy Link

I've used a **Note** to make it stand out more.

I've used a `**Note**` to make it stand out more.
README.md
justusbunsi (Migrated from gitea.com) commented 2023-12-11 17:22:44 +00:00
Copy Link

I wouldn't be specific about when to update. Every few years seems a bit long. 😉

- There is no need to update to every new PostgreSQL major version - you can happily skip some and do a larger update every few years.
+ There is no need to update to every new PostgreSQL major version - you can happily skip some and do larger updates when you are ready for them.
I wouldn't be specific about when to update. Every few years seems a bit long. 😉 ```diff - There is no need to update to every new PostgreSQL major version - you can happily skip some and do a larger update every few years. + There is no need to update to every new PostgreSQL major version - you can happily skip some and do larger updates when you are ready for them. ```
README.md
justusbunsi (Migrated from gitea.com) commented 2023-12-11 17:29:26 +00:00
Copy Link

We probably shouldn't refer to a OS-specific image. Otherwise, we should update the reference now and then. Maybe simply referring to :<majorVersion>?

We probably shouldn't refer to a OS-specific image. Otherwise, we should update the reference now and then. Maybe simply referring to `:<majorVersion>`?
justusbunsi (Migrated from gitea.com) commented 2023-12-12 13:56:27 +00:00
Copy Link

This is the image the Bitnami chart uses. I would prefer to link users to it to avoid them using the official PG image - for compatibility with the bitnami chart. There are reasons why Bitnami maintains their own PG images.

We could variabalize the 11 but I wouldn't want to be too generic here with the image recommendation.

This is the image the Bitnami chart uses. I would prefer to link users to it to avoid them using the official PG image - for compatibility with the bitnami chart. There are reasons why Bitnami maintains their own PG images. We could variabalize the `11` but I wouldn't want to be too generic here with the image recommendation.
justusbunsi (Migrated from gitea.com) commented 2023-12-12 17:41:49 +00:00
Copy Link

This is the image the Bitnami chart uses. I would prefer to link users to it to avoid them using the official PG image - for compatibility with the bitnami chart. There are reasons why Bitnami maintains their own PG images.

Absolutely. We should prevent any confusion. We could directly link to the DockerHub tag list site of the respective pgsql image. There is also the :<majorVersion> tag available.

> This is the image the Bitnami chart uses. I would prefer to link users to it to avoid them using the official PG image - for compatibility with the bitnami chart. There are reasons why Bitnami maintains their own PG images. Absolutely. We should prevent any confusion. We could directly link to the DockerHub tag list site of the respective pgsql image. There is also the `:<majorVersion>` tag available. - HA: https://hub.docker.com/r/bitnami/postgresql-repmgr/tags - Non-HA: https://hub.docker.com/r/bitnami/postgresql/tags
README.md
@ -121,7 +116,6 @@ Please double-check the image repository and available tags in the sub-chart:
- [PostgreSQL-HA](https://hub.docker.com/r/bitnami/postgresql-repmgr/tags)
- [PostgreSQL](https://hub.docker.com/r/bitnami/postgresql/tags)
justusbunsi (Migrated from gitea.com) commented 2023-12-11 17:32:42 +00:00
Copy Link

Intentional to use the permalink for redis-cluster but the main branch for pgsql-ha?
If not, this is the permalink for pgsql-ha: https://github.com/bitnami/charts/blob/b51288a469c1e42dac2ee2b3f26f1951eebf741f/bitnami/postgresql-ha/values.yaml#L106-L107

Intentional to use the permalink for redis-cluster but the main branch for pgsql-ha? If not, this is the permalink for pgsql-ha: `https://github.com/bitnami/charts/blob/b51288a469c1e42dac2ee2b3f26f1951eebf741f/bitnami/postgresql-ha/values.yaml#L106-L107`
justusbunsi (Migrated from gitea.com) commented 2023-12-12 13:57:18 +00:00
Copy Link

Good catch. Not intentional. I think the the dynamic link is better here. Yes, line numbers might change but linking to an old version might also confuse users.

Good catch. Not intentional. I think the the dynamic link is better here. Yes, line numbers might change but linking to an old version might also confuse users.
README.md
justusbunsi (Migrated from gitea.com) commented 2023-12-11 17:30:26 +00:00
Copy Link
- ... as they come in. ...
+ ... as they are released. ...
```diff - ... as they come in. ... + ... as they are released. ... ```
justusbunsi (Migrated from gitea.com) approved these changes 2023-12-13 16:55:05 +00:00
justusbunsi (Migrated from gitea.com) left a comment
Copy Link

LGTM 👍

LGTM 👍
Sign in to join this conversation.
Reviewers
No Reviewers
justusbunsi
Labels
Clear labels
has/backport
in progress
invalid
kind/breaking
kind/bug
kind/build
kind/dependency
kind/deployment
kind/docs
kind/enhancement
kind/feature
kind/lint
kind/proposal
kind/question
kind/refactor
kind/security
kind/testing
kind/translation
kind/ui
need/backport
priority/critical
priority/low
priority/maybe
priority/medium
reviewed/duplicate
reviewed/invalid
reviewed/wontfix
skip-changelog
status/blocked
status/needs-feedback
status/needs-reviews
status/wip
upstream/gitea
upstream/other
No Label
kind/docs
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: lunny/helm-chart#578
No description provided.
Delete Branch "refs/pull/578/head"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?