STORM-1175 Fixing close button, for Custom Port warning, in Preferences > Setup > Network


Submitter:	Ima Mechanique	Reviewers
Branch:		Groups:	viewer
Bugs:	STORM-1175	People:
Change Number:	None	Repository:	viewer-development

Description:

Applying Kitty Barnett's patch against current code base.

Testing Done:

Release build. tested preferences both in world and on the log in screen.

Expand All

Bugs Closed
- added STORM-1175

Description:

adding ticket reference

Could you please provide the patch description? I.e what the problem was and how you fixed it.

Ima Mechanique April 19, 2011, 8 p.m. (April 19, 2011, 8 p.m.)

I'm not sure what you want here.
If you're asking what I did? I updated Kitty's two line patch to apply against the current code, then tested that it fixed the problem.
If you're asking what Kitty's thinking was behind the solution? I can't answer that.

Vadim ProductEngine April 20, 2011, 8:56 a.m. (April 20, 2011, 8:56 a.m.)

Each commit (and the corresponding review request) should have a meaningful description, so that developers can [later] find out what the purpose of the code change was.
In this case it should have been something like "Fixed passing notification functor".

indra/llui/llnotifications.cpp (Diff revision 1)

LLNotificationsUtil is a wrapper around LLNotifications. Calling a wrapper from the class it wraps doesn't look very nice to me.

So I'd change this line to do the following:

    add(name.asString(), LLSD(), LLSD());

and remove the include.

Both approaches trigger a run-time warning though. I'll address that in a separate review request (coming soon).

Ima Mechanique May 10, 2011, 2:47 a.m. (May 10, 2011, 2:47 a.m.)

I've been waiting for the "separate review request" before continuing with this. Any idea when soon will be?

Vadim ProductEngine May 10, 2011, 9:13 a.m. (May 10, 2011, 9:13 a.m.)

Sorry, it seems to require more time than I thought, and I've been busy with other work.
So no, I'm not going to fix the warning issue in the near future. That should not prevent you from submitting your fix though.

Diff
- added Diff r2

Description:

Changes to patch per Vadim's reccomendation.

Thanks.

doc/contributions.txt (Diff revision 2)

OPEN-50 is unrelated, isn't it? Please clean your changesets.

Ima Mechanique May 11, 2011, 4:14 p.m. (May 11, 2011, 4:14 p.m.)
```
Oops, yes. Will fix that tomorrow.
```

Diff
- added Diff r3

Description:

Fixed the contributions entry, that sneaked in from my local repo, because it's missing in v-d.

Thanks Boroondas.

indra/llui/llnotifications.cpp (Diff revision 3)

Sorry, another change request ... Didn't see this at first:
Please remove the trailing whitespace.

No other objections, so ship it!

(Not quite seeing through that forest of add methods and Param constructors, so I just have to trust Vadim picked the right function call here.)

Diff
- added Diff r4

Description:

Removed trailing whitespace per Boroondas' request.

Yay, 4 RB revisions for a one-liner :-P

You have a pending review.

Other reviews

Your comment

Review Board 1.6.11

This change has been marked as submitted.