search

marcominerva / DatabaseGPT

Query a database using natural language
https://databasegpt.azurewebsites.net/
MIT License
59 stars 13 forks source link

Missing call to the method ThrowIfDisposed() #5

Closed N1K0232 closed 9 months ago

N1K0232 commented 9 months ago

I was watching the code and I noticed that in the SqlContext.cs object, there's a missing call to the ThrowIfDisposed method at line 37 Was in intentional or is it just missing?

marcominerva commented 9 months ago

Could you please add an hyperlink to the file you refer, so it is easy to identify it?

N1K0232 commented 9 months ago

Of course. My bad https://github.com/marcominerva/DatabaseGPT/blob/master/src/DatabaseGpt/DataAccessLayer/SqlContext.cs#L37

I specified the line but not the method name because I forgot it

marcominerva commented 9 months ago

Yes, it is definitively missing. In this particular case it isn't a problem, because the SqlContext class is used via Dependency Injection, but it is better to add the call to ThrowIfDisposed also in this method. Do you mind to take care of this task making a PR?

N1K0232 commented 9 months ago

Of course :)

N1K0232 commented 9 months ago

Question. The "review required" is enabled in this repository? I don't want to cause troubles when making the PR

marcominerva commented 9 months ago

Yes, just send the pull request as soon as you are ready

N1K0232 commented 9 months ago

Thank you so much for giving me the chance to collaborate :)