bman46 / InstagramEmbedDiscordBot

Embeds videos and images from an Instagram link into a Discord chat.
BSD 3-Clause "New" or "Revised" License
75 stars 25 forks source link

Refactoring #74

Closed yannikHoeflich closed 1 year ago

yannikHoeflich commented 1 year ago

My plan was to solve some issues. Some parts were very hard to find and many files are horrible long, partly through repetition of code.

I just started some refactoring, and much is still required to make the code more readable and better maintainable.

bman46 commented 1 year ago

Thanks @yannikHoeflich for the PR. A cleanup has been needed for awhile and I appreciate you taking on the task! I'll review this over the next couple days.

yannikHoeflich commented 1 year ago

Don't expect to much though. I just started a little bit with the SlashCommands and some, what I believe, are artifacts of older c# versions

bman46 commented 1 year ago

Reviewed code changes and they lgtm. Approved CodeQL runner and I will manually test before approving. Thanks again!

bman46 commented 1 year ago

Tested for functionality successfully. If you could review the CodeQL suggestions that would be great!

yannikHoeflich commented 1 year ago

Sure, most suggestions should be solved. But some of the suggestions are because of the generic try catch. Meant by that is:

try{
    // ...
} catch(Exception ex){
    // ...
}

In most cases you should catch specific Exceptions and continue the execution differently depending on the error. But in some cases, you just want to catch all, so I can not really do anything about the note.

bman46 commented 1 year ago

Sure, most suggestions should be solved. But some of the suggestions are because of the generic try catch. Meant by that is:

try{
    // ...
} catch(Exception ex){
    // ...
}

In most cases you should catch specific Exceptions and continue the execution differently depending on the error. But in some cases, you just want to catch all, so I can not really do anything about the note.

Yep, im fine just leaving them as is for now. Thanks!