Readme, #3038, #3041 #3042

JeanPaulLucien · 2022-02-15T13:11:55Z

While I have no idea ho to make a several PRs. You can select what to accept.

Fixed in Readme, added some Russian.
Disabled
Jabber: don't tell to server about Miranda version and OS by default #3038

Because it's unmainteained

I suppose this is workable solution

JeanPaulLucien

If it's lucky, it works.

tweimer · 2022-02-15T14:10:54Z

If it's lucky, it works.

Lucky? Did you compile it successfully and test your changes or not?

tweimer · 2022-02-15T14:12:24Z

While I have no idea ho to make a several PRs. You can select what to accept.

Well, you need to make a separate branch for each PR, and make one PR per branch.

tweimer

Once again, please compile and test your changes locally, otherwilse this PR will get closed.

tweimer · 2022-02-15T14:13:13Z

protocols/JabberG/src/jabber_caps.cpp

@@ -394,7 +394,7 @@ bool CJabberProto::HandleCapsInfoRequest(const TiXmlElement *, CJabberIqInfo *pI

 		CJabberClientPartialCaps *pCaps = g_clientCapsManager.GetPartialCaps(JABBER_CAPS_MIRANDA_NODE, m_szFeaturesCrc);
 		if (pCaps) {
-			if (m_bShowOSVersion) {
+			if (m_bShowOSVersion == true) {


Why that change?

@tweimer this to increase code stability.
@Ghazan able to not accept this change.

I don't see how it increases code stability.

How is it supposed to increase code stability?
It is just unnecessary.

In general, you better don't change code formatting on pull requests.
You better focus on the actual changes..

tweimer · 2022-02-15T14:13:20Z

protocols/JabberG/src/jabber_iq_handlers.cpp

@@ -41,7 +41,7 @@ bool CJabberProto::OnIqRequestVersion(const TiXmlElement*, CJabberIqInfo *pInfo)
 	query << XCHILD("name", "Miranda NG Jabber");
 	query << XCHILD("version", szCoreVersion);

-	if (m_bShowOSVersion) {
+	if (m_bShowOSVersion == true) {


Why that change?

tweimer · 2022-02-15T14:13:50Z

protocols/JabberG/src/jabber_opt.cpp

@@ -339,7 +339,8 @@ class CDlgOptAccount : public CJabberDlgBase
 	CCtrlCombo		m_cbResource;
 	CCtrlCheck		m_chkUseHostnameAsResource;
 	CCtrlCheck		m_chkUseDomainLogin;
-	CCtrlCombo		m_cbServer;
+	CCtrlEdit		m_txtServer; // "Domain/server" input
+	CCtrlCombo		m_cbServer; // Stay it here to don't get many errors


Remove it if it is no longer needed, and all other code that belongs to it.

protocols/JabberG/src/jabber_opt.cpp

tweimer · 2022-02-15T14:15:45Z

protocols/JabberG/res/jabber.rc

@@ -39,6 +39,7 @@ BEGIN
    PUSHBUTTON      "Cancel",IDCANCEL,223,207,50,14
 END

+// GUI in Main Menu, Accounts...


It's not useful to put a comment there, as Visual studio overrides it with auto-generated resource.

What is right way to add comment?

Just don't add one. I never saw one in the entire code.

@tweimer you comment looks like a personal opinion.

No, it's not. Do you see any other resource file in this repository with hand written comments?

@tweimer I asked you about argues, but you continue to ask questions to me. You joined to code review (https://en.wikipedia.org/wiki/Code_review).

You hold 10 strings about 2 weeks without argues about code. We did not improve Miranda, we did not know new about code. We did nothing (excluding @dartraiden who post helpful comments)

I did give you an argument in the very first comment about this. It's not useful to put a comment there, as Visual studio overrides it with auto-generated resource. Also, I did give you some helpful comments on how to improve your pull request.

If you prefer to argue instead of fixing those points, I will close here.

georgehazan

revert this please

georgehazan

m_bAllowVersionRequests has nothing to do with that
another fix is already applied

georgehazan · 2022-02-15T16:21:51Z

@JeanPaulLucien please separate code commits from langpack changes

tweimer · 2022-02-15T16:51:33Z

protocols/JabberG/src/jabber_proto.cpp

@@ -126,7 +126,7 @@ CJabberProto::CJabberProto(const char *aProtoName, const wchar_t *aUserName) :
 	m_bRosterSync(this, "RosterSync", false),
 	m_bSavePassword(this, "SavePassword", true),
 	m_bShowForeignResourceInMirVer(this, "ShowForeignResourceInMirVer", false),
-	m_bShowOSVersion(this, "ShowOSVersion", true),
+	m_bShowOSVersion(this, "ShowOSVersion", false),


That change is already done in e9dd5c1

Fixed losses letters. New function for the future optimization. Some cheats to avoid errors.

tweimer · 2022-02-23T07:01:13Z

Closing because:

You need to make a separate branch for each PR, and make one PR per branch. Don't add all commits to the same branch. There are a lot of helpful resources on google on how to do so.
You need to download Visual Studio and make sure your code compiles before submitting a pull request. There are obvious compile error which you see without even using a compiler. That just shows you did not put too much effort into your pull rerquest, It is not our job to fix your broken code.

JeanPaulLucien · 2022-02-23T07:44:54Z

There are obvious compile error which you see without even using a compiler. That just shows you did not put too much effort into your pull rerquest, It is not our job to fix your broken code.

@tweimer
So "obvious error" that you didn't say it. You are right about job. QA, research, code review and write code for Miranda is not my job. Farewell.

JeanPaulLucien added 11 commits February 12, 2022 08:44

Localization fixes

9bcb4e6

Added some Russian

dcae588

Fixed misses

ca2bb13

Fixed misses

9999eb3

Deleted link "Loist of public server"

e1b72be

Because it's unmainteained

Don't report OS versin to Jabber server by default

d347901

Delete the link "List of servers"

ed5e0ee

Deleted the link "List of servers" from GUI

3b35e9d

Domain?server does not upload server list

722a9ef

Domain/server input does not load server list

d1a5d07

I suppose this is workable solution

Don't report to server an OS info

290674b

JeanPaulLucien commented Feb 15, 2022

View reviewed changes

JeanPaulLucien added 2 commits February 15, 2022 15:30

Don't report OS info to server by default

1b36f3c

"Domain/server" input does not suggest servers

83484cd

tweimer requested changes Feb 15, 2022

View reviewed changes

georgehazan requested changes Feb 15, 2022

View reviewed changes

tweimer reviewed Feb 15, 2022

View reviewed changes

JeanPaulLucien added 3 commits February 15, 2022 21:23

Russian Miranda NG Readme.md

4cfa510

Make it readable

4d7cdba

Make "Domain/server" input

25b5d4a

Fixed losses letters. New function for the future optimization. Some cheats to avoid errors.

JeanPaulLucien requested a review from georgehazan February 15, 2022 19:59

tweimer closed this Feb 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Readme, #3038, #3041 #3042

Readme, #3038, #3041 #3042

JeanPaulLucien commented Feb 15, 2022

JeanPaulLucien left a comment

tweimer commented Feb 15, 2022

tweimer commented Feb 15, 2022

tweimer left a comment

tweimer Feb 15, 2022

JeanPaulLucien Feb 22, 2022

tweimer Feb 22, 2022

JeanPaulLucien Feb 23, 2022

tweimer Feb 23, 2022

tweimer Feb 15, 2022

JeanPaulLucien Feb 22, 2022

tweimer Feb 22, 2022

tweimer Feb 15, 2022

tweimer Feb 15, 2022

JeanPaulLucien Feb 22, 2022

tweimer Feb 22, 2022

JeanPaulLucien Feb 23, 2022

JeanPaulLucien Feb 23, 2022

tweimer Feb 23, 2022

JeanPaulLucien Feb 23, 2022

tweimer Feb 23, 2022

georgehazan left a comment

georgehazan left a comment

georgehazan commented Feb 15, 2022

tweimer Feb 15, 2022

tweimer commented Feb 23, 2022

JeanPaulLucien commented Feb 23, 2022

Readme, #3038, #3041 #3042

Readme, #3038, #3041 #3042

Conversation

JeanPaulLucien commented Feb 15, 2022

JeanPaulLucien left a comment

Choose a reason for hiding this comment

tweimer commented Feb 15, 2022

tweimer commented Feb 15, 2022

tweimer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgehazan left a comment

Choose a reason for hiding this comment

georgehazan left a comment

Choose a reason for hiding this comment

georgehazan commented Feb 15, 2022

Choose a reason for hiding this comment

tweimer commented Feb 23, 2022

JeanPaulLucien commented Feb 23, 2022