[Ovmsdev] New pull requests pending

Mon Aug 17 12:57:11 HKT 2020

Hi Mark,

I was trying to do a PR from my master branch as you suggest. My issue was that I've had separate 2 PRs so I had to do 2 branches and 1 PR from each branch. I didn't see any other possibility but I am no git expert and happy to learn. 

thx
Soko

On 17 August 2020 05:39:54 CEST, Mark Webb-Johnson <mark at webb-johnson.net> wrote:
>I see the discussion on the pull request itself is happening on the
>pull request and Michael is assisting.
>
>Regarding the process, I think in general we need to keep the core
>system stable and it is best not to commit partial implementations. I
>myself sometime violate that rule, but try not to. The better approach
>is to create a branch and do the work there (committing and sharing to
>others as you wish). Then, when all is done and ready, you can merge
>that branch in to master and submit a pull request. These branches can
>be either local (private) or on your GitHub clone (public). In rare
>cases, we also publish branches on the main openvehicles GitHub
>repository (for example, the SPIRAM branch that was there for a year
>before merging to master, and the upcoming 3.3 branch I am about to
>create for the changes to move from v3.2 -> v3.3).
>
>Regards, Mark.
>
>> On 14 Aug 2020, at 5:18 PM, Soko <ovms at soko.cc> wrote:
>> 
>> I see the point in not having unfinished code in the master so I
>removed my OBD implementation from this pull request.
>> Whats left is just the same source as before but with preparation for
>the OBD part.
>> 
>> @others: Please let me know your thoughts
>> 
>> On 14.08.2020 10:56, Chris van der Meijden wrote:
>>> I agree on not further discussing this. I made my point.
>>> 
>>> Chris
>>> 
>>> Am Freitag, den 14.08.2020, 10:02 +0200 schrieb Soko:
>>>> I'm not quite sure if we should discuss this here or in the pull
>request itself. Maybe Mark or Michael should say whats best. Anyhow...
>Let me put you to ease with my answers below...
>>>> 
>>>> On 14.08.2020 09:18, Chris van der Meijden wrote:
>>>>> Hey Soko,
>>>>> 
>>>>> sorry I don't agree on this pull request.
>>>> Just to make things clear with the other guys: You are talking
>about one of the pull requests, the one with the source split.
>>>>> 
>>>>> We have already discussed this.
>>>>> 
>>>>> Lets put this straight. My code for the T26A part is not ready
>yet. Climate control is still very buggy and it would confuse the user
>to have such a buggy feature within a possible OVMS release. It will
>take me probably a week or two, maybe longer, to get the climate
>control fixed. Then we can do a pull request to the master.
>>>> The T26A code in my pull request is not the one from your fork.
>It's the one that is officially in the master branch of openvehicles.
>So nothing changes for the user and not one source line is different
>then before.
>>>> 
>>>>> 
>>>>> An other point we also talked about, is that you have not
>documented your OBD part at all. This is also not acceptable. You can't
>put features with in the official OVMS repository that are not
>documented at all. How does the user use your VW e-Up OBD part? What
>hardware is needed? Where does the user see the results in the
>webfrontend? Does the app work with your code? ...
>>>> The user is not able to use any of the OBD code as pointed out in
>the pull request description. This is more an esthetic question as the
>code is not active. If the others feel the same I can remove my obd_*.*
>files from the pull request. The outcome is the same... 
>>>> 
>>>>> 
>>>>> You can't put unready code in the master.
>>>>> 
>>>>> That is the reason why we use forks. We merge a fork when we have
>kind of stable code with features that are well documented.
>>>>> 
>>>>> This is not the case at the moment with the splitted VW e-Up code.
>>>> To sum up: Nothing at all changes for the user with me my pull
>request. Well... besides the name of the VW e-Up vehicle. But is taken
>care of in the upgrade method.
>>>> 
>>>> I even prepared everything so you have it as easy as possible when
>this pull request is put into the master: When you pull it your fork
>you only have to backup your three t26_*.* files from your fork. Then
>just take 1:1 the openvehicles-master and put your backup back.
>>>> At least that's how I would do it as I'm not familiar how this is
>done in git directly ;)
>>>> 
>>>>> 
>>>>> Regards
>>>>> 
>>>>> Chris
>>>>> 
>>>>> 
>>>>> Am Freitag, den 14.08.2020, 07:06 +0200 schrieb Soko:
>>>>>> Hey guys,
>>>>>> 
>>>>>> Just want to let you know on this way as well about the two pull 
>>>>>> requests I have opened yesterday.
>>>>>> 
>>>>>> Let me know your thoughts. Would be cool if they will be merged
>very 
>>>>>> soon so I can continue my work on the VW e-Up.
>>>>>> 
>>>>>> regards,
>>>>>> 
>>>>>> Soko
>>>>>> 
>>>>>> _______________________________________________
>>>>>> OvmsDev mailing list
>>>>>> OvmsDev at lists.openvehicles.com
><mailto:OvmsDev at lists.openvehicles.com>
>>>>>> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
><http://lists.openvehicles.com/mailman/listinfo/ovmsdev>
>>>>> 
>>>>> 
>>>>> _______________________________________________
>>>>> OvmsDev mailing list
>>>>> OvmsDev at lists.openvehicles.com
><mailto:OvmsDev at lists.openvehicles.com>
>>>>> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
><http://lists.openvehicles.com/mailman/listinfo/ovmsdev>
>>>> _______________________________________________
>>>> OvmsDev mailing list
>>>> OvmsDev at lists.openvehicles.com
><mailto:OvmsDev at lists.openvehicles.com>
>>>> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
><http://lists.openvehicles.com/mailman/listinfo/ovmsdev>
>>> 
>>> 
>>> _______________________________________________
>>> OvmsDev mailing list
>>> OvmsDev at lists.openvehicles.com
><mailto:OvmsDev at lists.openvehicles.com>
>>> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
><http://lists.openvehicles.com/mailman/listinfo/ovmsdev>
>> _______________________________________________
>> OvmsDev mailing list
>> OvmsDev at lists.openvehicles.com
>> http://lists.openvehicles.com/mailman/listinfo/ovmsdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvehicles.com/pipermail/ovmsdev/attachments/20200817/2aa49842/attachment.htm>