laberning / openrowingmonitor

A free and open source performance monitor for rowing machines
https://laberning.github.io/openrowingmonitor
GNU General Public License v3.0
98 stars 19 forks source link

Add support for complex workouts, robustness improvements, bug fix #101

Closed JaapvanEkris closed 1 year ago

JaapvanEkris commented 1 year ago

This PR:

Abasz commented 1 year ago

@JaapvanEkris just a quick question, are you sure this PR is correct? there is an awful lot of commits here that I think are already on the v1beta branch and would cause duplication, while really you want the PR to include only the commits following the Add support for complex workouts commit.

JaapvanEkris commented 1 year ago

@Abasz I know, there is some odd GitHub behaviour. I made a pull towards V1Beta from the original backend_redesign baseline. I considered starting with a clean repo, but as you and Carlito are still based on that, I'm a bit puzzeled how to add this code.

ps: thanks for the review comments. Good points. Will fix the later on.

Abasz commented 1 year ago

Actually I used v1beta as basis for my changes, I did a rebase so my changes no longer depend on the Backend_redesign.

I am not sure why it has all the already merged commits but its possible that the issue could be that the original v1beta pull was a squash merge so from a git perspective it's one commit. So when you did the merg of v1beta to Backend_redesig you had the same code base but not the same history. I would try to create a new branch on v1beta (like development or something) than rebase the actual new commits.

Bit the above is just a guess.

JaapvanEkris commented 1 year ago

I would try to create a new branch on v1beta (like development or something) than rebase the actual new commits.

Is that possible? Because I wasn't able to create a new branch based on V1Beta. I tried, but I couldn't find how.

Abasz commented 1 year ago

Should be possible within your fork. You add the original repo as remote (I generally name these as upstream). Then pull the v1beta in, after checkout of which you should be able to create a new branch that points to the last commit of v1beta in your fork. You do the rebase (or cherry pick which ever you prefer) of the new commits and then it should be good to go.

I use vscode and git history add-on that helps a lot with the visualisation and checkouts etc.

JaapvanEkris commented 1 year ago

@Abasz : thanks! I fixed the issue with my branch and created a new clean branch for the changes.

@laberning : new update is ready in my repo. I hold back the update as @Abasz has some great ideas about cleaning up server.js, which are cool but require me to redo my homework on the session management (which I do with love, as it is a pretty cool approach).

JaapvanEkris commented 1 year ago

PR superseded by https://github.com/laberning/openrowingmonitor/pull/114