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

Folder Factories #3029

Closed

Conversation

AudriusButkevicius
Copy link
Member

@AudriusButkevicius AudriusButkevicius commented May 1, 2016

I always felt we were imprisoned by a boolean flag, hence this change.

Also, a bit of a longshot, but I am dropping the Folder Master business and calling things their real names, as I don't think Master has any better meaning than Read Only, actually the opposite, I think it causes for confusion as Master implies authoritative, which it isn't.

@calmh
Copy link
Member

calmh commented May 1, 2016

Haven't read the code yet but this sounds like a good idea. Probably negatory on the name change to read only though - that was the first name which literally no one understood. Take a look at the issue for sorting out the name, I think we settled on something like "send changes only" or so

@AudriusButkevicius
Copy link
Member Author

I don't understand how people don't understand it tho.

We also have the help text at the bottom, explaining what it is and a link to help, is that not enough?

@AudriusButkevicius
Copy link
Member Author

AudriusButkevicius commented May 1, 2016

As an alternative we can call it Read Only (Only send changes) in the dropdown, yet show Folder Type - Read Only?

@Zillode
Copy link
Contributor

Zillode commented May 1, 2016

Feels more like Write Only ;)

@lkwg82
Copy link
Contributor

lkwg82 commented May 1, 2016

Without reading it yet. My first reaction was "finally someone else had the same idea". (I already started a PR with the same intent.)

@AudriusButkevicius
Copy link
Member Author

Reverted back to master.

@@ -23,6 +23,9 @@ const (
OldestHandledVersion = 10
CurrentVersion = 13
MaxRescanIntervalS = 365 * 24 * 60 * 60

FolderTypeReadWrite = "readwrite"
Copy link
Contributor

Choose a reason for hiding this comment

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

These seem to fit better into folder.go (folderTypeXY to folder)

@calmh
Copy link
Member

calmh commented May 4, 2016

@st-review merge this, I'll fixup the type in next commit

@st-review
Copy link

👌 Merged as eabd2fc. Thanks, @AudriusButkevicius!

@st-review st-review closed this May 4, 2016
st-review pushed a commit that referenced this pull request May 4, 2016
@AudriusButkevicius AudriusButkevicius deleted the factories branch May 4, 2016 10:49
@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 Jun 16, 2017
@syncthing syncthing locked and limited conversation to collaborators Jun 16, 2017
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.

5 participants