birschick-bq commented on code in PR #2018:
URL: https://github.com/apache/arrow-adbc/pull/2018#discussion_r1739223818
##########
csharp/src/Drivers/Apache/Hive2/HiveServer2Connection.cs:
##########
@@ -27,21 +26,21 @@
namespace Apache.Arrow.Adbc.Drivers.Apache.Hive2
{
- public abstract class HiveServer2Connection : AdbcConnection
+ internal abstract class HiveServer2Connection : AdbcConnection
Review Comment:
@CurtHagenlocher
The motivation behind this change is ...
1. These Connection/Statement classes should not be creatable/visible
outside of the library. The entry point is/should be the Driver. We should go
back and change BigQuery - in my opinion.
2. In turn, this makes it "easier" to expose those methods that need to be
used outside the class. I was finding that I had to method-by-method find the
ones that needed to be set to `internal`.
3. This was also a best-practice for JDBC driver development I've worked on.
Of course, this is just a suggestion. If you feel it would make the PR
easier to review, I can revert these changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
