OLIMEX / DIY-LAPTOP

Do It Yourself Open Source Hardware and Software Modular Hacker's Friendly Laptop
Apache License 2.0
495 stars 89 forks source link

teres1-audioselect: cosmetic fixes #21

Closed khumarahn closed 5 years ago

khumarahn commented 6 years ago

Hi,

I tested the new version of teres1-audioselect as here: https://github.com/d3v1c3nv11/teres1-audioselect. It works nice. I reformatted it slightly (it was hard to read without e.g. consistent indentation). Nothing new or major, only improved readability.

Also, I removed the installation and start of the systemd service from the make install target. This is usually done separately, and not all systems run systemd.

jcstaudt commented 6 years ago

There are a decent number of changes to this file. I could merge it easier if each commit consisted of fewer changes (e.g. it is more difficult to merge files when content/formatting is adjusted and code is modified).

I made some (mostly formatting) adjustments that should make the conflict resolution a little easier. I may revisit this when I have some more time.

khumarahn commented 6 years ago

I think the new version is better. Except I don't like the readline business. See https://stackoverflow.com/questions/3501338/c-read-file-line-by-line. Why use the long uncorrected version from a question on stackoverflow, which does not properly work, instead of one in an answer:

    while ((read = getline(&line, &len, fp)) != -1) {
        printf("Retrieved line of length %zu :\n", read);
        printf("%s", line);
    }

Generally, using code from questions on stackoverflow should be illegal (code from answers may be OK).

Also, now eUnv.txt is not allowed to have empty lines. While this is naturally extected, this should not be an assumption in the long term.

jcstaudt commented 6 years ago

Sorry, I'm quite confused with your response. I didn't do any actual coding myself - only implemented some easy intermediate changes between master and your commit to make the merge easier. I agree with what you said about StackOverflow.

jcstaudt commented 5 years ago

How can I help get this PR resolved?

d3v1c3nv11 commented 5 years ago

This pull request can't be merged directly, but all ideas was implemented in teres1-audioselect.c The code was manually merged - conflict resolved.