GeezerVison

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

GeezerVison is showing github.com — Switch to Commands for basic example by inftord · Pull Request #13 · DSharpPlus/Example-Bots

Conversation

inftord commented Feb 1, 2024 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Member

i hate cnext.
basics done only for now, also updated dependencies for all projects for now

Sorry, something went wrong.

Uh oh!

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

All reactions

inftord added 2 commits

January 31, 2024 23:21


          update to .net 8; update d#+ to latest prerelease version

33dea9b


          update basics example to use Commands instead of CNext

98674db

inftord requested a review from OoLunar

February 1, 2024 18:01

OoLunar suggested changes

Feb 1, 2024

View reviewed changes

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.

Generally LGTM, just a few updates to the conventions I used. Does this PR delete the CNext examples?

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

src/DSharpPlus.Examples.Commands.Basics/Program.cs Resolved

Uh oh!

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

Comment thread

src/DSharpPlus.Examples.Commands.Basics/Program.cs Resolved

Uh oh!

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

Comment thread

src/DSharpPlus.Examples.Commands.Basics/Program.cs Resolved

Uh oh!

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

inftord added 3 commits

February 1, 2024 23:15


          murder CNext examples

c3aa229


          update to latest commands version

09af9a1


          do changes that Lunar requested

c50e6ff

inftord changed the title ~~Switch to Commands for examples~~ Switch to Commands for basic example

Feb 1, 2024

inftord marked this pull request as ready for review

February 2, 2024 03:17

inftord commented Feb 2, 2024

Copy link

Copy Markdown

Member Author

Generally LGTM, just a few updates to the conventions I used. Does this PR delete the CNext examples?

now it does

All reactions

Sorry, something went wrong.

Uh oh!

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

inftord requested a review from OoLunar

February 2, 2024 13:01

akiraveliara requested changes

Feb 2, 2024

View reviewed changes

Comment thread

src/DSharpPlus.Examples.Commands.Basics/Program.cs Outdated

		public static async Task Main(string[] args)
		{
		ConfigurationBuilder configurationBuilder = new();
		configurationBuilder.AddEnvironmentVariables("");

akiraveliara Feb 2, 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.

this is, at a glance, rather questionable. we should also probably encourage a config file to at least be considered

Sorry, something went wrong.

Uh oh!

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

All reactions

OoLunar Feb 2, 2024

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.

Config files... For an example bot? The purpose of an example bot is to demonstrate how to do XYZ while being easy to setup and run.

Also I think this might throw, if it hasn't been tested

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

src/DSharpPlus.Examples.Commands.Basics/Program.cs Outdated

		{
		DiscordClient client = new(new DiscordConfiguration()
		{
		Token = configuration.GetValue<string>("discord:token") ??

akiraveliara Feb 2, 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.

consider renaming "discord" in the eaxmples to "example-bot", to make clear what this identifier is

Sorry, something went wrong.

Uh oh!

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

All reactions

OoLunar Feb 2, 2024

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.

It's grabbing the discord token from the discord property:

{
   "discord":
   {
        "token": "invalid"
    }
}

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

src/DSharpPlus.Examples.Commands.Basics/Program.cs

		});
		IServiceProvider serviceProvider = serviceCollection.BuildServiceProvider();

		DiscordClient client = await serviceProvider.GetRequiredService<Task<DiscordClient>>();

akiraveliara Feb 2, 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.

why is this async

Sorry, something went wrong.

Uh oh!

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

All reactions

inftord Feb 2, 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.

asked by lunar in last review

Sorry, something went wrong.

Uh oh!

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

All reactions

akiraveliara Feb 2, 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.

@OoLunar why

Sorry, something went wrong.

Uh oh!

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

All reactions

Plerx2493 Feb 2, 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.

#13 (comment)
i think this comment is meant which was to a different part of the code

Sorry, something went wrong.

Uh oh!

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

All reactions

OoLunar Feb 2, 2024

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.

No, this is intended. Adding processors is an asynchronous operation. Encouraging .GetAwaiter().GetResult() goes against everything I've worked on within the lib, especially when it's done on ValueTasks.

Sorry, something went wrong.

Uh oh!

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

All reactions

Plerx2493 Feb 2, 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.

I would do the whole setup outside the AddSinglton and then add the client

Sorry, something went wrong.

Uh oh!

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

All reactions


          use json config by default instead of env vars

3162da3

OoLunar commented Feb 2, 2024

Copy link

Copy Markdown

Contributor

Generally LGTM, just a few updates to the conventions I used. Does this PR delete the CNext examples?

now it does

Please revert. We'll delete when CNext is officially deprecated

All reactions

Sorry, something went wrong.

Uh oh!

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


          Revert "murder CNext examples"

This reverts commit c3aa229.

inftord commented Feb 3, 2024

Copy link

Copy Markdown

Member Author

reverted

All reactions

Sorry, something went wrong.

Uh oh!

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

inftord requested a review from akiraveliara

February 3, 2024 18:16

JesperKuipers reviewed

Jun 4, 2024

View reviewed changes

Comment thread

src/DSharpPlus.Examples.Commands.Basics/Program.cs

		{
		ConfigurationBuilder configurationBuilder = new();
		// you can also use environment vars, just uncomment this
		// configurationBuilder.AddEnvironmentVariables("");

JesperKuipers Jun 4, 2024

Copy link

Copy Markdown

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.

// right click on config.json, click properties, and in Copy to Output Directory select Copy if newer

Sorry, something went wrong.

Uh oh!

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

All reactions

Reviewers

Plerx2493

Plerx2493 left review comments

OoLunar

Awaiting requested review from OoLunar

akiraveliara

Awaiting requested review from akiraveliara

+1 more reviewer

JesperKuipers

JesperKuipers left review comments

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

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

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

5 participants

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 01402503 — 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.

Conversation

inftord commented Feb 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OoLunar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

inftord commented Feb 2, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OoLunar commented Feb 2, 2024

Uh oh!

inftord commented Feb 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

inftord commented Feb 1, 2024 •

edited

Loading

Uh oh!

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