Skip to content
This repository has been archived by the owner on Dec 5, 2021. It is now read-only.

Events Feature from PR #679 #892

Closed
wants to merge 1 commit into from
Closed

Conversation

merqlove
Copy link

@merqlove merqlove commented Jan 9, 2020

Here is one commit changes from #679 PR. All changes is merged and cleaned. All dep's updates reverted.
If that project need such upgrade merge & change it for project needs.

.gitignore Outdated
@@ -221,3 +221,5 @@ FakesAssemblies/
**/*.Server/GeneratedArtifacts
**/*.Server/ModelManifest.xml
_Pvt_Extensions

somefuck
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

Copy link
Author

Choose a reason for hiding this comment

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

github not build if i doing force push :) i will squash all after changes

Copy link
Author

Choose a reason for hiding this comment

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

All questions fixed

README.md Outdated
request.Peer = peer;
client.SendRequestAsync<bool>(request).Wait();
}
catch {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

rather than a catch-all, can you please specify the type of exception here please

//logger.debug("processMessage: msg_id {0}, sequence {1}, data {2}", BitConverter.ToString(((MemoryStream)messageReader.BaseStream).GetBuffer(), (int) messageReader.BaseStream.Position, (int) (messageReader.BaseStream.Length - messageReader.BaseStream.Position)).Replace("-","").ToLower());
needConfirmation.Add(messageId);
Ack().Wait();
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this use await Ack();?

Copy link
Author

Choose a reason for hiding this comment

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

Error: The 'await' operator can only be used within an async method.
I will return it back, sorry limited by time

Copy link
Collaborator

Choose a reason for hiding this comment

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

then make processMessage be async

Copy link
Author

@merqlove merqlove Jan 9, 2020

Choose a reason for hiding this comment

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

Then here is 5 methods in a row, to be async

Copy link
Collaborator

Choose a reason for hiding this comment

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

sure?

Choose a reason for hiding this comment

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

the method Ack does not need to be async as the method it calls Send does not have any await calls itself. Should these still be marked async ?

@@ -42,6 +42,10 @@
<Reference Include="System.Data" />
<Reference Include="System.Net.Http" />
<Reference Include="System.Xml" />
<Reference Include="NLog">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm guessing the no-polling feature can exist without adding NLog, let's mix this change in this PR

Choose a reason for hiding this comment

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

you want NLog removed ?

}
}

private void _sender_UpdatesEvent (TLAbsUpdates updates)
Copy link
Collaborator

Choose a reason for hiding this comment

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

please use PascalCase for method names

{
var peer = new TLInputPeerUser() { UserId = status.UserId };
client.SendMessageAsync(peer, "Você está online.").Wait();
} catch {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this catch please

@knocte
Copy link
Collaborator

knocte commented Apr 13, 2020

Superseded by #940

@knocte knocte closed this Apr 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants