Move some internal board methods to protected

[Adimote]

Some internal methods for a board are public. namely: send, receive, close, and send_and_receive. This can cause problems with students, as they assume they might want to call the function.

Change the public methods of Board to be protected, to fix the aforementioned issue.

[Adimote]

Given that @thomasleese mentioned __ actually just obfuscates by renaming functions to their class names, I'd say a single _ is acceptable here. It's the difference between a student seeing _send_and_receive greyed out in Pycharm, and seeing _Board__send_and_receive, (and having to change whenever it is called in our code to the latter feels awful.)

[Adimote]

One possible argument against merging this is it's changing user-space for an arguably trivial reason.

[kierdavis]

One possible argument against merging this is it's changing user-space for an arguably trivial reason.

Although they currently are accessible to the user, these functions are undocumented and are only used internally, so I wouldn't consider them "userspace". Specifically, they're not "public API", which means we can change them however/whenever we see fit (regardless of whether a competitor has decided to use them).

[PeterJCLaw]

A single underscore is sufficient and appropriate here.

I don't think, we should be making close private. The others make sense to be changed.

I'm not worried about this changing userspace since these aren't documented anywhere (also it's early in the competition; I doubt anyone's doing anything advanced yet).

[PeterJCLaw]

@kierdavis I'm not sure I agree -- squashing commits like that makes the review much more complicated to follow as the offending commits have disappeared. It also means that any references to commits by id break(ish).

I appreciate the concerns around conflicts, however I've never personally hit that. Do you have an example PR which did hit that?

[kierdavis]

@kierdavis I'm not sure I agree -- squashing commits like that makes the review much more complicated to follow as the offending commits have disappeared. It also means that any references to commits by id break(ish).

That's a good point. One of the things I liked about using Gerrit was that it maintains the history of the review while allowing proposals to be amended instead of appended to. I'm not sure if Github are explicitly aiming to discourage using PRs in this manner, or if improving the UI is just low on their priority list.

I appreciate the concerns around conflicts, however I've never personally hit that. Do you have an example PR which did hit that?

Unfortunately not, though a similar problem cropped up once or twice during my summer employment. That job involved a lot of nasty merges and attempts to fit square features into round codebases, so this situation is probably out of the ordinary.