changes standard version to VT version 6

[dafiliks]

should resolve issue #374 if I understood correctly.

[GwnDaan]

Thanks for your PR, really appreciate it! I have a few comments, could you look at those?

[GwnDaan]

Also the version passed into the function is saved as a class member of the VirtualTerminalClient class, it'll be unused after these changes. So could you remove it aswell?

[dafiliks]

I'll try to do all these, thank you!

[dafiliks]

I have made another commit, check it out and let me know any feedback. Thanks.

[GwnDaan]

The build still seems to fail because where this function is used, the parameter "version" is still passed in: https://github.com/davidfiliks/AgIsoStack-plus-plus/blob/9f5e841d470eb57f8713a1e24accf06ecd5227f2/isobus/src/isobus_virtual_terminal_client.cpp#L1381

Furthermore, I was referring to this class member being unused after we make the changes in this PR: https://github.com/davidfiliks/AgIsoStack-plus-plus/blob/9f5e841d470eb57f8713a1e24accf06ecd5227f2/isobus/include/isobus/isobus/isobus_virtual_terminal_client.hpp#L1341
Would you be able to remove it, together with all places it is accessed/modified? That should only be inside the three set_object_pool functions.

I enabled the Github Actions checks to automatically start, try to get all checks to succeed. Again thanks for taking your time to make a contribution, highly appreciated :)

[dafiliks]

Thank you for helping like this, it is my first ever contribution on github. Check it out and give any feedback. Thanks.

[ad3154]

Thank you for helping like this, it is my first ever contribution on github. Check it out and give any feedback. Thanks.

Well, it's cool that your first contribution is here with us! Welcome!

It's looking pretty good to me, my only comment would be that it would be nice to squash or fixup all your commits into one commit with a nice tile, something like your first one "Change VT Client version to VT version 6" or something similar.

[GwnDaan]

It's looking pretty good to me, my only comment would be that it would be nice to squash or fixup all your commits into one commit with a nice tile, something like your first one "Change VT Client version to VT version 6" or something similar.

@ad3154 FYI, that's also something GitHub can do for us :). It's the Squash and merge option, though it indeed is a nice habit to keep commit history clean

[ad3154]

That's also something GitHub can do for us

I mean, yeah, I guess, it's just not the default on this repo so I forget it exists 😀

[dafiliks]

How would I merge all these commits into one?

[ad3154]

How would I merge all these commits into one?

It depends on how you interact with Git. I use fork but for raw Git, it would be something like what is described here with a git rebase -i HEAD~5 followed by force pushing the result to your branch.

This is a very common thing in Git, and fairly critical part of most git workflows, so it's something to look into as you gain experience with it, but no worries if it's not something you want to deal with right now, we can just have GitHub auto-squash it.

[dafiliks]

fatal: It seems that there is already a rebase-merge directory, and
I wonder if you are in the middle of another rebase. If that is the
case, please try
git rebase (--continue | --abort | --skip)
If that is not the case, please
rm -fr ".git/rebase-merge"
and run me again. I am stopping in case you still have something
valuable there.

I am not sure what to do here. This is the output after git rebase -i HEAD~5

[ad3154]

I am not sure what to do here. This is the output after git rebase -i HEAD~5

This seems like the rebase exited while it was happening, like maybe it couldn't open your selected default text editor or something, or there was some conflict... Don't worry about it for now, you can cancel it with git rebase --abort. We can just squash it on here.

[dafiliks]

Alright, thanks.