-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Consistently use spaces in C in Python tutorial #1418
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
[buildbot, ok to test] |
Much of the C that we are tracing originates from the Linux kernel, which uses tabs as indentation. Switching that C code to look like Python, with 4 character indents, I do not think looks right to a kernel engineer. The bcc repository has examples of both: some use tabs in their kernel code, and some uses 4 character indents. I'm fine with having examples of both, to appeal to different audiences. If I had to pick one, I'd switch the C code to tabs to be consistent with the kernel code that it is tracing, even though the remainder of the tool will be Python with 4 character indents. But I don't want to insist on one convention for all authors. I'd accept this change as it makes one file consistent -- the tutorial. But I would not like to see all the other tools in bcc changed. If a kernel engineer uses tabs in their C code, we should let them. If a python developer writes a bcc tool and wants 4 character indents everywhere, we should let them too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with Brendan.
Makes sense to keep C code kernel indented.
Sounds good to me. That makes sense. Will just change it to be standardised as kernel style. I think following the kernel's style guide then is probably the best solution. Some of the snippets just had both spaces and tabs in a single string of C which got me started down the rabbit hole. It might confuse people's IDEs if we have tabs and spaces together in Python, but the reasoning is sane and I'm happy with whichever cuz I'm a noob to C, as long as it's consistent within snippets, and ideally a file, but the mixing of C and Python in a single file is confusing and only for simple cases a lot of the time anyway. |
24653324.561322998 node 24728 path:/index.html | ||
24653335.343401998 node 24728 path:/images/welcome.png | ||
24653340.510164998 node 24728 path:/images/favicon.png | ||
TIME(s) COMM PID ARGS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what the tool actually outputs, we should change this, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, I didin't mean to edit any tool outputs.
LGTM. Could you collapse the commits to one? |
Awesome, thanks. Done |
@4ast could you take a look? |
Sorry, nitpick, but it was confusing me a bit while going through it.