All s300 vals #219
All s300 vals #219
Conversation
All s300 vals
rolls inS300 allvals
There was a problem hiding this comment.
Really nice work, thanks for this contribution.
Unfortunately, it's not that easy to merge this, first of all, because the tests are not passing (https://github.com/pablobuenaposada/HonDash/pull/219/checks), which is expected since you are sending more things through the websocket and tests need to be adjusted, no big, deal I will explain you what can we do with the issue of the tests.
The other thing is there are too many commits, normally you will expect only one commit per pull request, here we have around 30 with reverts in the middle that if I would merge it like this is going to pollute the git history.
Let's do something simpler, get one of those new values and just try to add it to the project, no frontend changes or other deviations, tests are most likely going to fail but I will cover them somehow, no problem, I just did something like this for the analogs that you can use it as example #222
Anyway, thanks, good job, I can fix all of this but I want your name on the commit and be more independent for next changes.
| @@ -0,0 +1,15 @@ | |||
| { | |||
There was a problem hiding this comment.
you can add the entire .vscode folder in gitignore so we don't push this type of files
There was a problem hiding this comment.
added .vscode/ to gitignore
docs/datajs_packet.txt
Outdated
| @@ -0,0 +1,73 @@ | |||
| {"data": { | |||
There was a problem hiding this comment.
this not needed to be commited
There was a problem hiding this comment.
No worries, this is resolved anyway, can delete altogether.
|
"The other thing is there are too many commits, normally you will expect only one commit per pull request, here we have around 30 with reverts in the middle that if I would merge it like this is going to pollute the git history." I'm good with this suggestion. What's the proper etiquette here? If I'm working on a branch for a while it's going to have at least a few commits. Do I finish, merge that branch back my master, and then submit a PR for the whole thing? Regarding paring down to only one added field, should I stash my larger changes locally, or just comment out the others until we are ready to roll them in? |
There's a way to squash commits before pull requesting, check google because is better explained there than I could do.
you can leave this pull request/branch here for now, my suggestion would be to just create a new branch from master and add manually whatever new value you pick and try to make a pull request, at least until you get a full workflow completed on how to add changes in here. |
|
Sounds good, thanks. I'll put some work into it today. |
pablobuenaposada
force-pushed
the
master
branch
5 times, most recently
from
75faa58 to
7d59981
Compare
pablobuenaposada
force-pushed
the
master
branch
5 times, most recently
from
ea0f7e3 to
6d0e6f1
Compare
@pablobuenaposada
Contains all S300v3 fields that I've been able to decode. Was having trouble showing all fields, a restart/rebuild fixed it.
All fields added were stimulated on the engine sim. I reserve the right to keep working on the conversion formulas :)
Please have a look and let me know your thoughts!