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

fix(device): always take apps input as source of truth #378

Merged
merged 1 commit into from
Feb 29, 2024
Merged

fix(device): always take apps input as source of truth #378

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions subcommands/common_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,19 +223,16 @@ func SetUpdatesConfig(opts *SetUpdatesConfigOptions, reportedTag string, reporte
configuredApps := sota.GetDefault("pacman.docker_apps", "").(string)
configuredTag := sota.GetDefault("pacman.tags", "").(string)

if len(configuredTag) == 0 && len(reportedTag) > 0 {
configuredTag = reportedTag
}
if len(configuredApps) == 0 && reportedApps != nil {
configuredApps = strings.Join(reportedApps, ",")
}
Copy link
Member

Choose a reason for hiding this comment

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

I believe that the reportedTag is subject to the same problem as reportedApps.
So, it'd be wise to update that logic accordingly as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. 👍

Copy link
Member

Choose a reason for hiding this comment

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

note - this is rolling back a "fix" - a8a21c7

Copy link
Member

Choose a reason for hiding this comment

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

or is it. i'm confused. can you add a little more to the commit message?

Copy link
Member

Choose a reason for hiding this comment

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

@doanac IIUC that fix is nice, but it introduced another bug we were discussing today.
So, Eric has now moved it to a better place (see lines below).

Copy link
Contributor

Choose a reason for hiding this comment

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

note - this is rolling back a "fix" - a8a21c7

I think that issue can be fixed by making fioconfig send the config to DG. It becomes possible with this change foundriesio/fioconfig#45.


changed := false
if opts.UpdateApps != "" && configuredApps != opts.UpdateApps {
if strings.TrimSpace(opts.UpdateApps) == "," {
opts.UpdateApps = ""
}
fmt.Printf("Changing apps from: [%s] -> [%s]\n", configuredApps, opts.UpdateApps)
fmt.Printf("Currently configured apps: [%s]\n", configuredApps)
if reportedApps != nil {
fmt.Printf("Apps reported as installed on device: [%s]\n", strings.Join(reportedApps, ","))
}
fmt.Printf("Setting apps to [%s]\n", opts.UpdateApps)
sota.Set("pacman.docker_apps", opts.UpdateApps)
sota.Set("pacman.compose_apps", opts.UpdateApps)
changed = true
Expand All @@ -244,7 +241,11 @@ func SetUpdatesConfig(opts *SetUpdatesConfigOptions, reportedTag string, reporte
if strings.TrimSpace(opts.UpdateTag) == "," {
opts.UpdateTag = ""
}
fmt.Printf("Changing tag from: %s -> %s\n", configuredTag, opts.UpdateTag)
fmt.Printf("Currently configured tag: %s\n", configuredTag)
if len(reportedTag) > 0 {
fmt.Printf("Tag reported by device: %s\n", reportedTag)
}
fmt.Printf("Setting tag to %s\n", opts.UpdateTag)
sota.Set("pacman.tags", opts.UpdateTag)
changed = true
}
Expand Down
Loading