| Message ID | 20230627181030.95608-2-irogers@google.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | parse-events clean up | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
> Removed by commit 70c90e4a6b2f ("perf parse-events: Avoid scanning > PMUs before parsing"). Will the chances ever grow to add another imperative change suggestion? See also: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94 Regards, Markus
On Fri, Jun 30, 2023 at 9:35 AM Markus Elfring <Markus.Elfring@web.de> wrote: > > > Removed by commit 70c90e4a6b2f ("perf parse-events: Avoid scanning > > PMUs before parsing"). > > Will the chances ever grow to add another imperative change suggestion? > > See also: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94 Sorry, I can't parse this. Thanks, Ian > Regards, > Markus
>>> Removed by commit 70c90e4a6b2f ("perf parse-events: Avoid scanning >>> PMUs before parsing"). >> >> Will the chances ever grow to add another imperative change suggestion? >> >> See also: >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94 > > > Sorry, I can't parse this. Can you take the requirement “Describe your changes in imperative mood” into account for any more descriptions? Regards, Markus
On Fri, Jun 30, 2023 at 10:15 AM Markus Elfring <Markus.Elfring@web.de> wrote: > > >>> Removed by commit 70c90e4a6b2f ("perf parse-events: Avoid scanning > >>> PMUs before parsing"). > >> > >> Will the chances ever grow to add another imperative change suggestion? > >> > >> See also: > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94 > > > > > > Sorry, I can't parse this. > > Can you take the requirement “Describe your changes in imperative mood” > into account for any more descriptions? Yep, still doesn't parse. Thanks, Ian > Regards, > Markus
>>>>> Removed by commit 70c90e4a6b2f ("perf parse-events: Avoid scanning >>>>> PMUs before parsing"). >>>> >>>> Will the chances ever grow to add another imperative change suggestion? >>>> >>>> See also: >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94 >>> >>> >>> Sorry, I can't parse this. >> >> Can you take the requirement “Describe your changes in imperative mood” >> into account for any more descriptions? > > Yep, still doesn't parse. Does this feedback really indicate that you stumble still on understanding difficulties for the linked development documentation? Can the mentioned patch review concern be adjusted with wording alternatives for improved commit messages? Regards, Markus
On Fri, Jun 30, 2023 at 10:23 AM Markus Elfring <Markus.Elfring@web.de> wrote: > > >>>>> Removed by commit 70c90e4a6b2f ("perf parse-events: Avoid scanning > >>>>> PMUs before parsing"). > >>>> > >>>> Will the chances ever grow to add another imperative change suggestion? > >>>> > >>>> See also: > >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94 > >>> > >>> > >>> Sorry, I can't parse this. > >> > >> Can you take the requirement “Describe your changes in imperative mood” > >> into account for any more descriptions? > > > > Yep, still doesn't parse. > > Does this feedback really indicate that you stumble still on understanding difficulties > for the linked development documentation? > > Can the mentioned patch review concern be adjusted with wording alternatives > for improved commit messages? Sorry, checked with a colleague and kernel contributor, we don't know what is being requested here, "imperative mood" makes no sense, as such I don't have a fix for what you're requesting. Thanks, Ian > Regards, > Markus
>> Can the mentioned patch review concern be adjusted with wording alternatives >> for improved commit messages? > > Sorry, checked with a colleague and kernel contributor, Interesting … > we don't know what is being requested here, Another bit of attention for a known information source: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94 > "imperative mood" makes no sense, How does such an opinion fit to the Linux development documentation? > as such I don't have a fix for what you're requesting. I got the impression that further possibilities can be taken better into account also for improved change descriptions. Regards, Markus
On Fri, Jun 30, 2023 at 10:52 AM Markus Elfring <Markus.Elfring@web.de> wrote: > > >> Can the mentioned patch review concern be adjusted with wording alternatives > >> for improved commit messages? > > > > Sorry, checked with a colleague and kernel contributor, > > Interesting … > > > > we don't know what is being requested here, > > Another bit of attention for a known information source: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94 > > > > "imperative mood" makes no sense, > > How does such an opinion fit to the Linux development documentation? > > > > as such I don't have a fix for what you're requesting. > > I got the impression that further possibilities can be taken better into account > also for improved change descriptions. Thanks Markus, I appreciate you feel you have a real point here, I'm just not getting it. Perhaps you can write a commit message that fulfils requirements like being in the correct "imperative mood" and I will learn and improve. Thanks, Ian > Regards, > Markus
>> I got the impression that further possibilities can be taken better into account >> also for improved change descriptions. > > Thanks Markus, I appreciate you feel you have a real point here, > I'm just not getting it. I hope that remaining communication difficulties can be resolved somehow. > Perhaps you can write a commit message that fulfils requirements Let us take another look. > like being in the correct "imperative mood" You chose some imperative wordings already for other changes while the description for this update step is still improvable. > and I will learn and improve. I hope also that further contributors and patch reviewers will “get into the mood” to share any additional ideas for more desirable change descriptions. How do you think about a wording approach like the following? Token specifications were adjusted by the commit 70c90e4a6b2fbe775b662eafefae51f64d627790 ("perf parse-events: Avoid scanning PMUs before parsing"). It was noticed that the token “PE_PMU_EVENT_FAKE” was not so useful any more. Thus delete its usage here. Regards, Markus
On Fri, Jun 30, 2023 at 10:05:22AM -0700, Ian Rogers wrote: > On Fri, Jun 30, 2023 at 9:35 AM Markus Elfring <Markus.Elfring@web.de> wrote: > > > > > Removed by commit 70c90e4a6b2f ("perf parse-events: Avoid scanning > > > PMUs before parsing"). > > > > Will the chances ever grow to add another imperative change suggestion? > > > > See also: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94 > > > Sorry, I can't parse this. Markus is banned from vger. Just ignore him. regards, dan carpenter
>>> See also: >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.4#n94 >> >> Sorry, I can't parse this. > > Markus is banned from vger. Just ignore him. I hope that further chances will picked up for desirable improvements despite of questionable communication difficulties. Regards, Markus
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index 9f28d4b5502f..64755f9cd600 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -63,7 +63,7 @@ static void free_list_evsel(struct list_head* list_evsel) %token PE_LEGACY_CACHE %token PE_PREFIX_MEM PE_PREFIX_RAW PE_PREFIX_GROUP %token PE_ERROR -%token PE_KERNEL_PMU_EVENT PE_PMU_EVENT_FAKE +%token PE_KERNEL_PMU_EVENT %token PE_ARRAY_ALL PE_ARRAY_RANGE %token PE_DRV_CFG_TERM %token PE_TERM_HW @@ -81,7 +81,7 @@ static void free_list_evsel(struct list_head* list_evsel) %type <str> PE_MODIFIER_EVENT %type <str> PE_MODIFIER_BP %type <str> PE_EVENT_NAME -%type <str> PE_KERNEL_PMU_EVENT PE_PMU_EVENT_FAKE +%type <str> PE_KERNEL_PMU_EVENT %type <str> PE_DRV_CFG_TERM %type <str> name_or_raw name_or_legacy %destructor { free ($$); } <str> @@ -394,44 +394,6 @@ PE_KERNEL_PMU_EVENT opt_pmu_config YYABORT; $$ = list; } -| -PE_PMU_EVENT_FAKE sep_dc -{ - struct list_head *list; - int err; - - list = alloc_list(); - if (!list) - YYABORT; - - err = parse_events_add_pmu(_parse_state, list, $1, /*head_config=*/NULL, - /*auto_merge_stats=*/false); - free($1); - if (err < 0) { - free(list); - YYABORT; - } - $$ = list; -} -| -PE_PMU_EVENT_FAKE opt_pmu_config -{ - struct list_head *list; - int err; - - list = alloc_list(); - if (!list) - YYABORT; - - err = parse_events_add_pmu(_parse_state, list, $1, $2, /*auto_merge_stats=*/false); - free($1); - parse_events_terms__delete($2); - if (err < 0) { - free(list); - YYABORT; - } - $$ = list; -} value_sym: PE_VALUE_SYM_HW
Removed by commit 70c90e4a6b2f ("perf parse-events: Avoid scanning PMUs before parsing"). Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/parse-events.y | 42 ++-------------------------------- 1 file changed, 2 insertions(+), 40 deletions(-)