adding bot variables - Githubissues

The1Moxie commented 3 years ago

added the config variables to make the bot connect as well as added a class to maintain non-public information.
added logging support

The1Moxie commented 3 years ago

there are github secrets for sensitive info

Now you are not making sense I blocked the class in the gitignore it's not included.

cgytrus commented 3 years ago

there are github secrets for sensitive info

Now you are not making sense I blocked the class in the gitignore it's not included.

Yeah, but why would you add a class for it if there's already a built-in feature made specifically for that in GitHub?

The1Moxie commented 3 years ago

because I am a Github noob. and didn't know

LukasRH commented 3 years ago

This approach is actually the correct one to take. You have a class representation of your config which you then load values into when the bot starts. This makes it a lot easier to get the values, and you don't have to continually make environment call to get the values.

The approach is to have the class representation with a property for each "entry" you want to have in your config. This class should contain no values, and be included in the project. You then have either a JSON file or some other means of storage, that you will load into memory on startup and then cast this information into an instance of the config class you made. JSON is often used for this, as it can be directly parsed into a class instance without any formatting.

So what is wrong in this PR, and i mean no offense at all.

The config class have been left out, which means that other developers cloning will need to guess how it would look.
GitHub secrets... This is only used internally on GitHub, when running a pipeline like the one that test if the code can be compiled. When locally on a pc, we do not have access to these secrets. Therefore it cant be used for development nor production.

cgytrus commented 3 years ago

oh, didn't know that about github secrets, sorry. But the class should be still either not included or have loading from a file implemented, because then you'd have to not commit it even though, for example in my IDE, I can select what files to commit and it'd be always just sitting there annoying me and most likely other developers

LukasRH commented 3 years ago

@cgytrus I think you need to read what I wrote again. And as for not committing a file, that is what gitignore is for. so you don't have a file "just sitting there".

cgytrus commented 3 years ago

@LukasRH ok wait yeah i'm dumb, also I just realized that not committing the file would fail the CI build, so the file should be committed and the config should be loaded from a file

The1Moxie commented 3 years ago

Not sure if this is what you wanted it was a bit outside my scope.

RhythmGamesDev / RhythmGamesBot

adding bot variables #26