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

gui, lib/config: Add notification about new release channels #3945

Closed
wants to merge 3 commits into from
Closed

gui, lib/config: Add notification about new release channels #3945

wants to merge 3 commits into from

Conversation

calmh
Copy link
Member

@calmh calmh commented Jan 30, 2017

Purpose

As above

Testing

Clicketyclick

Screenshots

screen shot 2017-01-30 at 23 12 09

The link goes to https://build.syncthing.net/docs/223/users/releases.html

@@ -200,7 +200,7 @@ func TestOverriddenValues(t *testing.T) {
AlwaysLocalNets: []string{},
OverwriteRemoteDevNames: true,
TempIndexMinBlocks: 100,
UnackedNotificationIDs: []string{},
UnackedNotificationIDs: []string{"channelNotification"}, // added in 17->18 migration
Copy link
Member

Choose a reason for hiding this comment

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

Suggest you break this over multiple lines so that we end up with:

[]string{
  "foo", //bar
  "baz". //nuf
}

@AudriusButkevicius
Copy link
Member

@st-review lgtm

@st-review
Copy link

@AudriusButkevicius: Noted! Need another LGTM or explicit merge command.

@@ -0,0 +1,15 @@
<configuration version="17">
Copy link
Member

Choose a reason for hiding this comment

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

v18?

Copy link
Member

Choose a reason for hiding this comment

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

Its 17 to 18 migration

Copy link
Member

Choose a reason for hiding this comment

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

Ok. I was simply comparing to v17.xml where the numbers matched, no "deeper insights" :)

Copy link
Member Author

Choose a reason for hiding this comment

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

No you're right it should be 18. Fixed. Also another thing, the override test was flaky.

@calmh
Copy link
Member Author

calmh commented Feb 4, 2017

@st-review merge it

@st-review
Copy link

👌 Merged as 3eb7a93. Thanks, @calmh!

@st-review st-review closed this Feb 4, 2017
st-review pushed a commit that referenced this pull request Feb 4, 2017
@calmh calmh deleted the prerel-notif branch February 4, 2017 12:21
@st-review st-review added the pr-merged Legacy label used in the past for pull requests merged to the main tree label Jan 15, 2018
@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 Jul 15, 2018
@syncthing syncthing locked and limited conversation to collaborators Jul 15, 2018
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 pr-merged Legacy label used in the past for pull requests merged to the main tree
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants