Skip to content

feat: upgrade urfavre cli to v3 - #6483

Open
Planlos5000 wants to merge 1 commit into
mainfrom
feat/fs/upgrade_urfave_cli_v3
Open

feat: upgrade urfavre cli to v3#6483
Planlos5000 wants to merge 1 commit into
mainfrom
feat/fs/upgrade_urfave_cli_v3

Conversation

@Planlos5000

@Planlos5000 Planlos5000 commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Description

Upgrade the urfavre cli package to v3.

Changelog

  • Upgrade package
  • Adjust tests

Checklist

  • I have performed a self-review of my own code
  • I have written and understand every part of this contribution myself - if AI tools were used, I have thoroughly reviewed and verified all changes
  • I have commented my code, particularly in hard-to-understand areas

(Optional) Visual Changes

@Planlos5000 Planlos5000 self-assigned this Aug 10, 2026
@Planlos5000 Planlos5000 added Review Needed This pull request is ready for review feat This pull request or issue adds a new feature labels Aug 10, 2026
@Planlos5000
Planlos5000 force-pushed the feat/fs/upgrade_urfave_cli_v3 branch from cc7e9f0 to 5a332d6 Compare August 11, 2026 14:13
@mating02
mating02 self-requested a review August 12, 2026 09:42
mating02
mating02 reviewed Aug 12, 2026
View reviewed changes
Comment thread server/src/go.mod
@Planlos5000
Planlos5000 force-pushed the feat/fs/upgrade_urfave_cli_v3 branch 2 times, most recently from f221579 to 53eecd2 Compare August 14, 2026 12:39
mating02
mating02 requested changes Aug 17, 2026
View reviewed changes
Comment thread server/src/main.go Outdated
Name: "auth-google-client-id",
Sources: cli.NewValueSourceChain(
cli.EnvVar("SCRUMLR_AUTH_GOOGLE_CLIENT_ID"),
toml.TOML("auth-callback-host", altsrc.NewStringPtrSourcer(&tomlconfigFile)),

@mating02 mating02 Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looks like a copy and paste error, why auth-callback-host here? I think it should be auth-google-client-id

@Planlos5000 Planlos5000 Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed that

Comment thread server/src/main.go
&cli.BoolFlag{
Name: "disable-check-origin",
Sources: cli.NewValueSourceChain(
toml.TOML("disable-check-origin", altsrc.NewStringPtrSourcer(&tomlconfigFile)),

@mating02 mating02 Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why no environment variable support for disable-check-origin here?

@Planlos5000 Planlos5000 Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Because the user should set it explicit, because it has some security implications

Comment thread server/src/main.go
EnvVars: []string{"SCRUMLR_ENABLE_EXPERIMENTAL_AUTH_FILE_SYSTEM_STORE"},
},
&cli.BoolFlag{
Name: "auth-enable-experimental-file-system-store",

@mating02 mating02 Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this name here seems to be wrong. in the toml file, it is auth-experimental-file-system-store

@Planlos5000 Planlos5000 Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I have changed the name in the toml file since the env var is also called SCRUMLR_ENABLE_EXPERIMENTAL_AUTH_FILE_SYSTEM_STORE

Comment thread server/src/main.go
Name: "auth-enable-experimental-file-system-store",
Sources: cli.NewValueSourceChain(
cli.EnvVar("SCRUMLR_ENABLE_EXPERIMENTAL_AUTH_FILE_SYSTEM_STORE"),
toml.TOML("auth-enable-experimental-file-system-store", altsrc.NewStringPtrSourcer(&tomlconfigFile)),

@mating02 mating02 Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

also here, i think the enable in between needs to be removed or the name in the toml needs to be changed

Comment thread server/config_example.toml Outdated
otel-http = ""

# Enable or disable verbose logging.
verbose = true

@mating02 mating02 Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

where is this flag used?

@Planlos5000 Planlos5000 Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Verbose flag was replaced with the log level. It was deprecated with version 4.0.0 and was then removed with version 4.5.0. This was just never removed from the toml file

@Planlos5000
Planlos5000 force-pushed the feat/fs/upgrade_urfave_cli_v3 branch from 53eecd2 to 147c95b Compare August 18, 2026 06:16
@Planlos5000
Planlos5000 force-pushed the feat/fs/upgrade_urfave_cli_v3 branch from 147c95b to 38eb5bb Compare August 21, 2026 07:16
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

The deployment to the dev cluster was successful. You can find the deployment here: https://6483.development.scrumlr.fra.ics.inovex.io
This deployment is only for testing purposes and will be deleted after 1 week.
To redeploy rerun the workflow.
DO NOT STORE IMPORTANT DATA ON THIS DEPLOYMENT

Deployed Images

  • ghcr.io/inovex/scrumlr.io/scrumlr-frontend:sha-778635b

  • ghcr.io/inovex/scrumlr.io/scrumlr-server:sha-778635b

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

Reviewers

@mating02 mating02 mating02 requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@Planlos5000 Planlos5000

Labels

feat This pull request or issue adds a new feature Review Needed This pull request is ready for review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Planlos5000 @mating02