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.

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

lib/model: Fixed adding empty items to device list (fixes #8646) #8647

Merged
merged 1 commit into from
Dec 6, 2022

Conversation

Ratio2
Copy link
Contributor

@Ratio2 Ratio2 commented Nov 5, 2022

Purpose

Fixed an obvious typo that resulted in empty entries in the device list.

Testing

Didn't do any testing.

For testing, you need to create 3 devices, connect 1 and 2, make 1 introducer for 2, connect 1 and 3, make sure that device 3 automatically appears on 2.

Documentation

No need to change documentation.

Authorship

Your name and email will be added automatically to the AUTHORS file
based on the commit metadata.

Copy link
Member

@calmh calmh left a comment

Choose a reason for hiding this comment

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

Honestly I'm not sure what's going on here to begin with, with the copying, but assuming that it's necessary for a good reason, the whole

cfg.Folders = make([]config.FolderConfiguration, 0, len(folders))
for _, fcfg := range folders {
    cfg.Folders = append(cfg.Folders, fcfg)
}

can be replaced with the somewhat shorter

cfg.Folders = append([]config.FolderConfiguration{}, folders...)

and the same for devices...

@calmh
Copy link
Member

calmh commented Dec 6, 2022

(Fixed since)

@calmh calmh closed this Dec 6, 2022
@rasa
Copy link
Member

rasa commented Dec 6, 2022

@calmh Fixed how?
https://github.com/Ratio2/syncthing/blob/09e4b0108397e6b682ef7ce12ae788fc0f8d65b9/lib/model/model.go#L1275-L1282 still shows:

			cfg.Folders = make([]config.FolderConfiguration, 0, len(folders))
			for _, fcfg := range folders {
				cfg.Folders = append(cfg.Folders, fcfg)
			}
			cfg.Devices = make([]config.DeviceConfiguration, len(devices))
			for _, dcfg := range devices {
				cfg.Devices = append(cfg.Devices, dcfg)
			}

@calmh
Copy link
Member

calmh commented Dec 6, 2022

Ah, it hasn't, I fooled myself by being on the PR branch when looking at it 🤦 Thanks for catching that.

@calmh calmh reopened this Dec 6, 2022
@calmh calmh merged commit 0573800 into syncthing:main Dec 6, 2022
@calmh calmh added this to the v1.22.3 milestone Dec 13, 2022
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Dec 7, 2023
@syncthing syncthing locked and limited conversation to collaborators Dec 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants