GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — build DiscordClient on top of IOC by akiraveliara · Pull Request #1908 · DSharpPlus/DSharpPlus

Conversation

@akiraveliara

@akiraveliara akiraveliara commented May 17, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

current status:

  • RestClient capable of construction
  • Gateway incapable of construction
    • dispatch constructed and extensible through IOC
    • possibly the raw gw client extensible through IOC?
  • expand capability of DiscordClientBuilder to allow configuring logging and services
  • process events through DiscordClientBuilder
  • obsolete old events (on DiscordClient only, since this won't support sharding yet)
  • docs

non-goals of this PR:

  • sharded client support (future PR)
  • extension support (future PR)
  • exposing more extensibility interfaces (future PR, open issues for each)
  • ratelimiting constructed and extensible through IOC (future PR)

leave issues for any other features associated with this you may want

closes #1683
makes progress towards #1820

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
akiraveliara added 2 commits May 17, 2024 11:35
@akiraveliara
centralize the service provider
4d85806
@akiraveliara
prepare the rest client for IOC
4f42a50
@akiraveliara akiraveliara added enhancement core epic-ioc for pull requests and issues relating to IOC in v5 labels May 17, 2024
@akiraveliara akiraveliara added this to the v5.0 milestone May 17, 2024
VelvetToroyashi
VelvetToroyashi reviewed May 17, 2024
View reviewed changes

@VelvetToroyashi VelvetToroyashi left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM aside from comments; gonna open an issue wrt some tomfoolery happening regarding ratelimit handling

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread DSharpPlus/Extensions/ServiceCollectionExtensions.InternalSetup.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread DSharpPlus/Net/Rest/RestClient.cs
Comment on lines +74 to +75
this.httpClient.BaseAddress = new Uri(Utilities.GetApiBaseUri());
this.httpClient.Timeout = timeout;

@VelvetToroyashi VelvetToroyashi May 17, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Perhaps it'd be worthwhile to go the Remora route and expose a [rest] client builder; there's occasional reason to change what the client does (e.g., adding additional polly handlers for metrics, or using the canary api for one reason or another)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@VelvetToroyashi VelvetToroyashi May 17, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

(Client builder as in Action<HttpClient> parameter)

https://github.com/Remora/Remora.Discord/blob/87adfc57b1a0980234713f6c77a1e2be25f5567e/Backend/Remora.Discord.Gateway/Extensions/ServiceCollectionExtensions.cs#L54-L59

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@akiraveliara akiraveliara May 30, 2024

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

i think this is something we should look into in a separate pull request to customize specifically rest further

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@VelvetToroyashi VelvetToroyashi mentioned this pull request May 17, 2024
Rest client ratelimit inconsistencies #1909
Closed
akiraveliara added 5 commits May 17, 2024 20:29
@akiraveliara
obsolete all the old events...
eaf6a5e
@akiraveliara
obsolete SocketErrored even more
1b0e386
@akiraveliara
enables registering event handlers to the service collection
6c22cd2
@akiraveliara
fix event-related build errors
6fc8ed7
@akiraveliara
make DiscordApiClient IOC-constructible
1e62cd4
@akiraveliara akiraveliara mentioned this pull request May 21, 2024
v5 mega-issue #1580
Open
19 of 31 tasks
@akiraveliara
death (migrate events (not dispatch yet) to the interim model)
5c8f2c4
@Plerx2493 Plerx2493 mentioned this pull request May 25, 2024
Rework reconnection logic #1921
Closed
akiraveliara added 6 commits May 26, 2024 17:22
@akiraveliara
update DiscordClient.Dipshit.cs
b2ebdae
@akiraveliara
fix most trivial build errors
905ce60
@akiraveliara
more progress towards usability
68675df
@akiraveliara
DSharpPlus.dll should now work:tm:
32035b6
@akiraveliara
build?
5a3e002
@akiraveliara
miser, miser
af572a6
@akiraveliara
akiraveliara marked this pull request as ready for review May 29, 2024 19:47
@akiraveliara akiraveliara added the big-change label May 29, 2024
akiraveliara added 3 commits May 29, 2024 22:20
@akiraveliara
add public construction code
06a7058
@akiraveliara
Merge branch 'master' into aki/discordclient-ioc
0fc1bbd
@akiraveliara
update docs
7284840
@akiraveliara
akiraveliara requested review from OoLunar, Plerx2493 and VelvetToroyashi May 30, 2024 15:20
@akiraveliara
convenience
edd2984
OoLunar
OoLunar approved these changes May 30, 2024
View reviewed changes

@OoLunar OoLunar left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think this is okay? Regardless please write a migration guide for this PR specifically. Before we release v5, we'll merge the migration guide for this PR and all other breaking changes into one. I just want a guide to reference right now since I myself don't particularly understand what the intended use is.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread DSharpPlus/Clients/DiscordClient.Dispatch.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread DSharpPlus/Clients/DiscordClient.Events.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread DSharpPlus/Clients/DiscordClient.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
VelvetToroyashi
VelvetToroyashi approved these changes May 31, 2024
View reviewed changes

@VelvetToroyashi VelvetToroyashi left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Cursory glance (namely because of a jumble of a diff), but LGTM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
akiraveliara added 2 commits May 31, 2024 12:23
@akiraveliara
details
ec97bdc
@akiraveliara
migration guide (partial, only as far as this PR is concerned)
bfef5fc
@akiraveliara akiraveliara added docs labels May 31, 2024
@akiraveliara

akiraveliara commented May 31, 2024

Copy link
Copy Markdown
Member Author

@OoLunar added the migration guide as requested
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
akiraveliara added 2 commits May 31, 2024 15:05
@akiraveliara
build
8948ab0
@akiraveliara
merge conflicts
a6de76c
OoLunar
OoLunar approved these changes Jun 2, 2024
View reviewed changes
Comment thread docs/articles/migration/4x_to_5x.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread docs/articles/migration/4x_to_5x.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
akiraveliara added 2 commits June 2, 2024 21:06
@akiraveliara
switch to CreateDefault
72efde2
@akiraveliara
nit
989eab0
@akiraveliara
akiraveliara merged commit 613936b into master Jun 2, 2024
@akiraveliara
akiraveliara deleted the aki/discordclient-ioc branch June 2, 2024 19:09
This was referenced Jun 3, 2024
Rework command filtering for processor specific commands #1914
Merged
v5 breaking changes tracking issue #1585
Open
@akiraveliara akiraveliara mentioned this pull request Jul 7, 2024
add sharding support to DiscordClient #1971
Merged
@ecrocombe ecrocombe mentioned this pull request Jul 9, 2024
Commands not being registered #1986
Closed
OoLunar pushed a commit that referenced this pull request Oct 3, 2024
@akiraveliara
build DiscordClient on top of IOC (#1908)
44a40d8 
* centralize the service provider

* prepare the rest client for IOC

* obsolete all the old events...

* obsolete SocketErrored even more

* enables registering event handlers to the service collection

* fix event-related build errors

* make DiscordApiClient IOC-constructible

* death (migrate events (not dispatch yet) to the interim model)

* update DiscordClient.Dipshit.cs

* fix most trivial build errors

* more progress towards usability

* DSharpPlus.dll should now work:tm:

* build?

* miser, miser

* add public construction code

* update docs

* convenience

* details

* migration guide (partial, only as far as this PR is concerned)

* build

* switch to CreateDefault

* nit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@VelvetToroyashi VelvetToroyashi VelvetToroyashi approved these changes
@Plerx2493 Plerx2493 Awaiting requested review from Plerx2493
+1 more reviewer
@OoLunar OoLunar OoLunar approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

big-change core docs enhancement epic-ioc for pull requests and issues relating to IOC in v5

Projects

None yet

Milestone

v5.0

Development

Successfully merging this pull request may close these issues.

Centralized ServiceProvider

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@akiraveliara @VelvetToroyashi @OoLunar
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.