Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

renamed ltc_encoder_set_bufsize and ltc_encoder_get_buffer to more descriptive/consistent names #48

Closed
wants to merge 1 commit into from

Conversation

superbigio
Copy link

PR for #45

}

int ltc_encoder_set_buffersize(LTCEncoder *e, double sample_rate, double fps) {
free (e->buf);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use tabs for indent, space to align. Prefer to keep whitespace changes separately. It makes it hard to see the actual diff here.

I should probably push the whole thing though clang-format at some point.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about that. I have my compiler (Xcode) setup to turn tabs into spaces automatically. Many people don't want tabs in their code. Be aware that filter and volume getters have also been affected by different whitespace formatting. I only realized it when I looked at the code online. Again sorry about that.

Yes, doing a clang-format is a good idea indeed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The convention you are following is to indent with one tab = 8 spaces, correct?

@x42
Copy link
Owner

x42 commented Nov 9, 2019

Looks good in general. I might split this up in two commits when rebasing (NO-OP: whitespace ;; and API-addition/deprecation)

@superbigio
Copy link
Author

I am not sure what to do from here. Should I send you a new PR with the correct formatting or are you gonna merge this one and then re-format later?

@x42
Copy link
Owner

x42 commented Nov 10, 2019

I've manually applied the changes (63542ba), also updated references in the documentation to point to new functions and updated the example-code, too.

@x42 x42 closed this Nov 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants