Closed lars18th closed 2 years ago
Hi @Barracuda09 ,
Before sharing my changes related to #163 I suggest to merge this. My development environment uses an old compiler and this solves the problem to "recover" the lost compatibility with ancient platforms. I hope you agree with this solution. I feel it's clear and easy to maintain. So it doesn't need to generate troubles with your regular development. The "new" code is only enabled with the NEED_BACKPORT that is not enabled by default. My idea is time to time, check new changes and provide a fix if they will be required. All tests are done using GCC 6.3 and the program runs like a charm.
You agree? 😉
Hi @Barracuda09 ,
I need to know your opinion about the backport compatibility. I have two options:
Please, could you answer what you prefer?
Take note that with this PR all the project compiles and runs without any trouble with GCC 6.3. So my idea is to maintain almost this basic compatibility.
Or use a patch file?
Maybe use somethink like this?
Create patch with:
git format-patch backport --stdout > backport.patch
And apply with:
git am backport.patch
Hi @Barracuda09 ,
That's the option 1. Remember that any PR can be downloaded as a patch adding ".diff" at the en of the URL. So this one is: https://patch-diff.githubusercontent.com/raw/Barracuda09/SATPI/pull/164.diff
Therefore, if you leave the backport version X open, any user can download it and apply it. We only need to publish a simple how to in the BUILD section of the README. You agree with this then?
Anyway, I'll try to push the merge of some irrelevant and easy changes, like the proposed in #167 . Please, merge it!
Hi @Barracuda09 ,
This PR is today up-to-date. Therefore, please don't close it. Any user that needs support for non-C++17 compilers can download it (with wget https://github.com/Barracuda09/SATPI/pull/164.diff
) and apply it (patch -p1 < 164.diff
) to create a compatible version.
I'll try in the future to check it and maintain it updated.
Hi @lars18th
You could add something to the makefile that handles this?
Hi @Barracuda09
You could add something to the makefile that handles this?
You want that the current Makefile handles this patch? That's what I've already implemented. However, perhaps what you want is to apply the patch automatically? It's this? Then, I need to know your opinion to this question:
Regards.
I think as a diff file from the repository, that way you do not have to wait on me to add it.
Edit: But the Makefile I can add
Hi @Barracuda09 ,
To support the last changes I've created a new PR that supersedes this #173 . You agree with the idea to close this and continue the discussion in the another onether one?
Hi @Barracuda09 ,
I think as a diff file from the repository, that way you do not have to wait on me to add it. Edit: But the Makefile I can add
So, your idea is then:
backport-c11.diff
file that we merge into the master repo (perhaps in a subdir called backport).Makefile
to apply the patch and compile if we call to for example with make backport-c11
.You want this?
Hi @lars18th
Yes that is exactly what I mean.
Hi @Barracuda09 ,
If you will merge the PR #175 then please close this one.
New pull request #173 does handle this
Enable backport compatibility with compilers previous to C++17.
List of changes:
Tested with GCC 6.3