[PATCH v2] trace-cmd: Add CODING_STYLE and CONTRIBUTE documents

* [PATCH v2] trace-cmd: Add CODING_STYLE and CONTRIBUTE documents
@ 2021-04-06 15:37 Steven Rostedt
  2021-04-06 15:38 ` Steven Rostedt
  0 siblings, 1 reply; 2+ messages in thread
From: Steven Rostedt @ 2021-04-06 15:37 UTC (permalink / raw)
  To: Linux Trace Devel; +Cc: Sameeruddin shaik

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Add a CODING_STYLE document that describes the expected style of the code
as well as a CONTRIBUTE document that explains how a new comer can
contribute to the project.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 CODING_STYLE | 287 +++++++++++++++++++++++++++++++++++++++++++++++++++
 CONTRIBUTE   | 103 ++++++++++++++++++
 2 files changed, 390 insertions(+)
 create mode 100644 CODING_STYLE
 create mode 100644 CONTRIBUTE

diff --git a/CODING_STYLE b/CODING_STYLE
new file mode 100644
index 00000000..24fb10ec
--- /dev/null
+++ b/CODING_STYLE
@@ -0,0 +1,287 @@
+
+trace-cmd coding-style
+======================
+
+The coding style of trace-cmd and the tracing libraries (libtracefs and
+libtraceevent) are very similar to the Linux kernel coding style:
+
+  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst
+
+Indentation
+===========
+
+Tabs are used for the start of indentation (the '\t' character), and should be
+set to 8 characters. Spaces may be used at the end for continued lines where
+having the start of text line up to braces in the previous line is not
+divisible by 8.
+
+Max line width
+==============
+
+All lines should not be more than 100 characters in length.
+
+This is a guide, as readability is more important than breaking lines up into a
+hard limit. Ideally, strings should never be broken up except for where a new
+line is added.
+
+	printf("This is a line that may continue for a very long string.\n"
+	       "This is another line, but after a new line\n");
+
+But line breaks should not be:
+
+	printf("This is a line that may continue for a very"
+	       "long string.\n This is another line,"
+	       "but after a new line\n");
+
+Not only is the above not as readable as the first version, it is not
+even equivalent, because it is missing spaces between the line breaks.
+For this reason, finish the string on the same line, even if that string
+breaks the 100 character limit.
+
+Brackets and braces
+===================
+
+For all conditionals, the braces start on the same line:
+
+	if (cond) {
+	}
+
+And the ending brace is at the same indentation as the conditional.
+
+	while (cond) {
+	}
+
+	do {
+	} while (cond);
+
+	for (i = 0; i < 10; i++) {
+	}
+
+The same is true for structures:
+
+	struct my_struct {
+		int field;
+	};
+
+But for functions, the braces should start on the following line:
+
+	void my_function(void)
+	{
+	}
+
+
+It is also fine to not use braces for simple conditionals and loops.
+
+	if (!x)
+		y = x;
+	else
+		y = 1;
+
+	for (i = 0; i < 10; i++)
+		foo(i);
+
+	while (getline(&line, &size, fp) > 0)
+		printf("%s", line);
+
+But any complex or multiline conditional or loop should have braces even if it
+is allowed not to by the C language.
+
+	if (x) {
+		for (i = 0; i < 10; i++)
+			foo(i);
+	} else {
+		foo(1);
+	}
+
+Notice above that even though the else portion is simple, it too has braces as
+the else and if blocks should match. If one is required to have braces, they
+both should have braces.
+
+
+Spaces
+======
+
+A single space should be used between C commands and their starting
+parenthesis.
+
+	if (x)
+	for (i = 0; i < 10; i++)
+	while (getline(&line, &size, fp) > 0)
+
+There should be no space between function or macros and the starting
+parenthesis.
+
+	foo(x)
+	IS_VALID(y)
+
+This includes prototypes and declarations.
+
+	void foo(int x)
+
+A space should be before and after assignment, comparison and algorithmic
+signs.
+
+	i = 0;
+	if (i < 10)
+	if (i == 5)
+
+	y = i + 10;
+
+	i += 5;
+
+For structures, use tabs to make all the fields line up nicely.
+
+	struct {
+		int			foo;
+		int			bar;
+		unsigned long long	time;
+	};
+
+Variable declarations
+=====================
+
+The order of variables that are declared, should first keep the same types
+together, but also should be ordered by their length such that the variables
+are ordered in an "upside-down Christmas tree" fashion where the length gets
+smaller.
+
+	int tracecmd_count_cpus(void)
+	{
+		static int once;
+		char buf[1024];
+		int cpus = 0;
+		char *pbuf;
+		size_t *pn;
+		FILE *fp;
+		size_t n;
+		int r;
+
+The above shows that the order is done by length, and in the above example it
+also shows that "int cpu = 0;" is not grouped next to "int r;". As this is more
+of a guideline and made to be more aesthetic to the eye of the reader, both the
+above and is acceptable as below.
+
+	int tracecmd_count_cpus(void)
+	{
+		static int once;
+		char buf[1024];
+		char *pbuf;
+		size_t *pn;
+		FILE *fp;
+		size_t n;
+		int cpus = 0;
+		int r;
+
+
+Unless variables are tightly related, it is expected that each variable be on
+its own line and not grouped by type. That is,
+
+		int r, cpus = 0;
+
+is to be discouraged, as the two variables are not related to each other.
+But if you had a bunch of counters:
+
+		int i, j, k;
+
+That would be fine, as the variables are all related as they are all for the
+same purpose (arbitrary counters). The same may go with pointers;
+
+
+	char *begin, *end;
+
+Comments
+========
+
+Comments will use the "/* */" format and the C++ "//" style is discouraged.
+If a comment is on one line, keep the "/*" and "*/" on the same line:
+
+	/* This is a single line comment. */
+
+If a comment spans more than one line, then have the "/*" on a separate line
+before the comment and the "*/" on a separate line at the end of the comment,
+and each line starts with a "*" where all the "*" line up with each other.
+
+	/*
+	 * This is a multi line comment, where all the '*'
+	 * will line up, and the text is on a separate line
+	 * as the start and end markers.
+	 */
+
+
+Function documentation
+======================
+
+All global functions (and especially any APIs) should have a function
+description in the form of "kernel doc":
+
+  https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html
+
+The form is:
+
+  /**
+   * function_name() - Brief description of function.
+   * @arg1: Describe the first argument.
+   * @arg2: Describe the second argument.
+   *        One can provide multiple line descriptions
+   *        for arguments.
+   *
+   * A longer description, with more discussion of the function function_name()
+   * that might be useful to those using or modifying it. Begins with an
+   * empty comment line, and may include additional embedded empty
+   * comment lines.
+   *
+   * The longer description may have multiple paragraphs.
+   *
+   * Context: Describes whether the function can sleep, what locks it takes,
+   *          releases, or expects to be held. It can extend over multiple
+   *          lines.
+   * Return: Describe the return value of function_name.
+   *
+   * The return value description can also have multiple paragraphs, and should
+   * be placed at the end of the comment block.
+   */
+
+Structure layout
+================
+
+This is more about compaction than coding style. When creating structures, be
+aware that if the fields are placed together without being sized by alignment,
+that the compiler will create "holes" in them.
+
+	struct {
+		int			x;
+		char			y;
+		unsigned long long	f;
+	};
+
+As int is 4 bytes in length, char is one byte, and unsigned long long is 8
+bytes. The compiler will try to naturally align them by their size, and will
+include padding (holes) inside the structure to do so. The above is equivalent
+to:
+
+	struct {
+		int			x;
+		char			y;
+		char			padding[3];
+		unsigned long long	f;
+	};
+
+It is best to try to organize the structure where there are no holes within
+them.
+
+	struct {
+		unsigned long long	f;
+		int			x;
+		char			y;
+	};
+
+The above is better formatting, even if there may be padding outside the
+structure, but the compiler will still have more flexibility to utilize the
+space outside the structure than what it can do within it.
+
+General
+=======
+
+As stated, this is a guide and may not be strictly enforced. The goal is to
+have consistent and readable code. In general, try to have the coding style
+match the surrounding code.
diff --git a/CONTRIBUTE b/CONTRIBUTE
new file mode 100644
index 00000000..aed78110
--- /dev/null
+++ b/CONTRIBUTE
@@ -0,0 +1,103 @@
+If you like to become part of the community and submit patches, here's how
+to do so for trace-cmd.
+
+If you only want to report a bug, or suggest an enhancement, you may do
+so at:
+
+  https://bugzilla.kernel.org/buglist.cgi?component=Trace-cmd%2FKernelshark
+
+All development is done via a mailing list:
+
+   http://vger.kernel.org/vger-lists.html#linux-trace-devel
+
+Patches should be sent to linux-trace-devel@vger.kernel.org
+
+Start by cloning the official repository:
+
+  git clone git://git.kernel.org/pub/scm/utils/trace-cmd/trace-cmd.git
+
+Make your changes. When you are satisfied with them, commit them into git.
+Here's some helpful hints for your git commits.
+
+1) When making changes, please follow the coding style defined by the file
+   called CODING_STYLE in this directory.
+
+2) Every commit should only do one thing.
+   That is, if your work requires some cleaning up of code, do that
+   clean up as a separate commit and not with your functional changes.
+   Find ways to take "steps" in modifying code. If you can break up
+   your changes in a series of steps, do so.
+
+3) The commit log should start with a title. Like the below
+
+      trace-cmd: Add CONTRIBUTE file
+
+   Even though this repo is for trace-cmd, start the topic with
+   "trace-cmd:" because the commits will end up as patches to a mailing
+   list that handles other tracing repos, differentiating them with the subject
+   is useful. You can be more specific as well. If the change only affects the
+   "record" command, you may start the title with "trace-cmd record:".
+
+4) The body of the commit (with a blank line from the title), should be self
+   contained, and explain why you are making the change. The title should hold
+   the "what" is changing, but the body contains the rationale for the change.
+   It should be a stand alone, and not state things like "See the next patch",
+   because when it is in git history, there's no knowing what the next patch
+   is.  You can make statements like "This is needed for a <future-feature>
+   that will come later". Where "<future-feature>" is something that you are
+   working on and the current commit is one of the steps required to get there.
+
+5) Add your Developer Certificate of Origin (DCO) at the bottom of the commit
+   log. That is "Signed-off-by: Full Name <email>" where your full name is your
+   real name (no pseudonyms). Optionally, if you are making the change on
+   behalf of your company, you may also add your company name, if you are not
+   using your company's email. "Signed-off-by: Full Name (Company) <email>".
+   Please note, the DCO is your statement that you have the legal right to
+   make these changes for the project you are submitting to.
+
+You can use the Linux kernel "checkpatch.pl" script to help verify the formatting
+of your patch:
+
+  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/checkpatch.pl
+
+Please note that checkpatch.pl is a guide and not a hard rule. If it reports a
+fix that makes the code harder to read, that fix can probably be ignored.
+
+  git format-patch --stdout HEAD~1..HEAD | ./checkpatch.pl
+
+Finally, you can use the git "send-email" functionality:
+
+  git --send-email --from='<your-email> --to='linux-trace-devel@vger.kernel.org' HEAD~1..HEAD
+
+If you are sending one patch, if you are adding more than one patch, also include
+a cover letter:
+
+  git --send-email --cover-letter --annotate --from='<your-email> --to='linux-trace-devel@vger.kernel.org' <first-commit>~1..HEAD
+
+If you receive feedback on your patches, and plan on sending another version,
+please use the '-v' option to mark your patches that they are a new version.
+For example, if you add "-v2" to the above commands, instead of having:
+"[PATCH]" in the subject, it will have "[PATCH v2]", letting the reviewers know
+that this is a new version. If you send another version, use "-v3" and so on.
+
+For more information about git --send-email:
+
+  https://git-scm.com/docs/git-send-email
+
+To keep track of the status of patches that have been submitted, check out:
+
+  https://patchwork.kernel.org/project/linux-trace-devel/list/
+
+If you would like to apply patches from the mailing list, you can use
+the "b4" utility.
+
+ $ pip install b4
+
+Then from the mailing list archive, find a message id from a patch or patch
+series. For example, to get the patch from:
+
+  https://lore.kernel.org/linux-trace-devel/20210205173713.132051-1-tz.stoyanov@gmail.com/
+
+ $ b4 am -o - 20210205173713.132051-1-tz.stoyanov@gmail.com > /tmp/p.mbox
+ $ git am /tmp/p.mbox
+
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread