> In the case that nothing matches, you can always use commit --no-verify to
> skip that verification step.
I am not a fan of this solution. If it is not something entirely new, it must
match. Otherwise, a new tag should be proposed, along with the sequence of
patches that use it.
> Maybe when we push to gerrit we can have a message pop up saying "you may
> want to tag the following people in your review:..."?
This is a bit more tricky: by doing it when the commit is created, there is a
chance that the push stage will not be done on the same day (we hope things are
tested before being sent for review), and therefore the people to be tagged
will be forgotten. Ideally this should be done on push, but I haven't looked
into git push hook parameters yet to know how "intrusive" the hook code would
be.
Regards,
Daniel
Em quarta-feira, 23 de outubro de 2019 00:40:43 GMT+2, Jason Lowe-Power
<[email protected]> escreveu:
We should add systemc as a tag and set Gabe as the maintainer :).
I like the idea of the default to be a strict set of tags. This would force
the contributor to think about who should be tagged in the review as well
(it would be great if we could automate this!). In the case that nothing
matches, you can always use commit --no-verify to skip that verification
step.
Maybe when we push to gerrit we can have a message pop up saying "you may
want to tag the following people in your review:..."?
Cheers,
Jason
On Tue, Oct 22, 2019 at 3:35 PM Gabe Black <[email protected]> wrote:
> I'm not sure if there's a lot of value in having a very strictly set
> defined set of tags. That's sort of (but not entirely) like having a fixed
> set of email subject lines. Sure, it makes it easier to group things, but
> not everything will fit in the predefined categories. I think needless
> differences (arch-sparc vs. sparc vs. whatever) would be nice to get rid
> of, but I think this should maybe more of a warning thing than a hard
> barrier.
>
> And if somebody ignores the warnings/suggestions for no good reason, that's
> what reviews are for :-).
>
> Gabe
>
> On Tue, Oct 22, 2019 at 12:37 PM Bobby Bruce <[email protected]> wrote:
>
> > It seems like there may be some value in adding "systemc" as an official
> > tag, but, from what I can see, most of the other incorrect tags are
> mapped
> > pretty cleanly to existing ones.
> >
> > Would anyone have any objection to adding "systemc" as a tag in
> > MAINTAINERS?
> >
> > In regards to server-side hooks: it sounds like it'd be good but a
> > client-side hook also has value, so I'm still ok with either.
> >
> > Kind regards,
> > Bobby
> >
> > --
> > Dr. Bobby R. Bruce
> > Room 2235,
> > Kemper Hall, UC Davis
> > Davis,
> > CA, 95616
> > ________________________________
> > From: gem5-dev <[email protected]> on behalf of Daniel Carvalho
> <
> > [email protected]>
> > Sent: Saturday, October 19, 2019 6:38 AM
> > To: gem5 Developer List <[email protected]>
> > Subject: Re: [gem5-dev] gem5 tags rework
> >
> > Thank you for the info, Nikos. Here are the updated values (2400 commits,
> > around May of 2017, a few days after the MAINTAINERS file creation):
> >
> > {'systemc': 466, 'arch-arm': 251, 'mem-cache': 193, 'cpu': 142, 'mem':
> > 128, 'dev-arm': 127, 'base': 108, 'arm': 97, 'sim': 84, 'tests': 83,
> 'dev':
> > 68, 'mem-ruby': 63, 'python': 62, 'arch': 60, 'x86': 59, 'scons': 57,
> > 'configs': 53, 'sim-se': 52, 'config': 45, 'misc': 44, 'arch-riscv': 43,
> > 'util': 32, 'sparc': 26, 'cpu-o3': 23, 'riscv': 20, 'arch-x86': 19,
> 'ext':
> > 18, 'stats': 14, 'ruby': 14, 'alpha': 13, 'kvm': 13, 'mips': 13,
> > 'system-arm': 12, 'power': 12, 'learning_gem5': 11, 'syscall_emul': 11,
> > 'gpu-compute': 10, 'fastmodel': 10, 'style': 7, 'ps2': 7, 'tlm': 5,
> 'kern':
> > 4, 'arch-mips': 3, 'null': 3, 'cpu-minor': 3, 'gpu': 3, 'net': 3,
> > 'arch-power': 3, 'hsail': 3, 'arch-hsail': 2, 'cpu-tester': 2, 'pwr': 2,
> > 'hsail-x86': 2, 'virtio': 2, 'isa': 2, 'arch-alpha': 1, 'o3': 1,
> > 'arch-sparc': 1, 'testlib': 1, 'probe': 1, 'gdb': 1, 'sconstruct': 1,
> > 'vnc': 1, 'cpu-kvm': 1, 'm5': 1, 'testers': 1, 'test': 1, 'hmc': 1,
> 'docs':
> > 1, 'mem-garnet': 1, 'arch-generic': 1, 'learning-gem5': 1, 'tarch': 1,
> > 'pl011': 1}
> >
> >
> > Matches:
> >
> > {'arch-arm': 251, 'mem-cache': 193, 'cpu': 142, 'mem': 128, 'base': 108,
> > 'sim': 84, 'tests': 83, 'dev': 68, 'mem-ruby': 63, 'python': 62, 'arch':
> > 60, 'scons': 57, 'configs': 53, 'sim-se': 51, 'misc': 44, 'arch-riscv':
> 43,
> > 'util': 31, 'cpu-o3': 23, 'arch-x86': 19, 'ext': 18, 'stats': 14,
> > 'system-arm': 12, 'gpu-compute': 10, 'arch-mips': 3, 'arch-power': 3,
> > 'cpu-minor': 3, 'arch-hsail': 2, 'arch-alpha': 1, 'cpu-kvm': 1,
> > 'arch-sparc': 1, 'mem-garnet': 1, 'learning-gem5': 1}
> >
> > Unused tags:
> >
> > ['dev-virtio', 'system-alpha', 'sim-power', 'system', 'cpu-simple']
> >
> > Although the pattern remains, and there is still a significant amount of
> > non-'arch-' tags, having the 'arch-' prefix would solve the 'power'
> > ambiguity issue, and would comply with this recent decision, and the
> > directory names. Therefore, I suggest keeping the prefix. If this is
> > adopted, IIRC, 'ruby' would be the only tag that does not comply with
> > directory names, therefore 'mem-ruby' should be enforced instead.
> >
> >
> > A server side check is still not being done (just started reading about
> > the hooks), but it is doable, and way more desirable than a client side,
> > since it can be easily skipped.
> >
> >
> > Regards,
> > Daniel
> > Em sábado, 19 de outubro de 2019 13:47:20 GMT+2, Nikos Nikoleris <
> > [email protected]> escreveu:
> >
> > Hi Daniel,
> >
> > If I remember correctly, the rules on commit tags changed about 2.5
> > years ago (see also git log MAINTAINERS) along with the migration from
> > mercurial to git. Previously tags such as arch-arm and arch-x86 were
> > simply arm and x86. In addition tags such as dev-arm, systemc were added
> > more recently as the result of the creation (or refactoring) of a new
> > subsystem.
> >
> > I think you might have to limit the window in your analysis to see if
> > and what tags we use consistently.
> >
> > It would be nice if we could do this check and other style checks
> > through a commit hook or a server side check and not to have to rely on
> > reviews.
> >
> > Regards,
> >
> > Nikos
> >
> >
> > On 19/10/2019 06:54, Daniel Carvalho wrote:
> > > Hello all,
> > >
> > > I have recently proposed adding a git hook (
> > https://gem5-review.googlesource.com/c/public/gem5/+/21739) to make sure
> > that commit messages follow the guidelines in CONTRIBUTING, with the
> > uttermost goal being to make sure that the gem5 tags used are valid. I
> have
> > also checked if the previous commits would have been valid if this hook
> had
> > been applied, and just a tiny percentage of them would,
> > > Although typos were frequent, the main issue was that the tags in
> > MAINTAINERS do not correspond to reality, and many other tags were being
> > used instead.
> > >
> > > I have done a quick check on all used tags since around 2013 (last 5000
> > commits) to check which and how many times each tag has been used. I have
> > manually removed some noise (fixed simple tag typos, and ignored
> grotesque
> > outliers, which were less than 0.1% of the tags), and all tags were
> treated
> > as lowercase. This is the result:
> > >
> > > {'mem': 575, 'systemc': 466, 'cpu': 368, 'arm': 347, 'ruby': 327,
> > 'arch-arm': 251, 'sim': 214, 'stats': 194, 'mem-cache': 193, 'config':
> 193,
> > 'dev': 192, 'base': 187, 'x86': 172, 'tests': 144, 'scons': 142,
> 'dev-arm':
> > 127, 'misc': 113, 'arch': 93, 'kvm': 79, 'python': 73, 'util': 70,
> > 'configs': 67, 'mem-ruby': 63, 'syscall_emul': 53, 'sim-se': 52, 'style':
> > 48, 'ext': 48, 'gpu-compute': 46, 'arch-riscv': 43, 'riscv': 35, 'sparc':
> > 32, 'power': 26, 'cpu-o3': 23, 'alpha': 21, 'arch-x86': 19, 'mips': 18,
> > 'slicc': 17, 'hsail': 16, 'regressions': 13, 'system-arm': 12, 'o3': 11,
> > 'learning_gem5': 11, 'syscall-emul': 10, 'fastmodel': 10, 'isa': 8, '':
> 7,
> > 'ps2': 7, 'kern': 7, 'test': 6, 'dist': 5, 'tlm': 5, 'pwr': 5, 'cache':
> 4,
> > 'energy': 4, 'gpu': 4, 'probe': 3, 'virtio': 3, 'arch-mips': 3, 'cpu/o3':
> > 3, 'net': 3, 'cpu-minor': 3, 'syscall emulation': 3, 'revert "cpu': 3,
> > 'arch-power': 3, 'null': 3, 'build': 3, 'copyright': 2, 'swig': 2,
> > 'arch-sparc': 2, 'x86 regressions': 2, 'cpu-tester': 2, 'loader': 2,
> > 'arch-hsail': 2, 'cpuid': 2, 'minor': 2, 'o3 cpu': 2, 'syscall': 2,
> > 'proto': 2, 'vnc': 2, 'hsail-x86': 2, 'arch-alpha': 1, 'base simple cpu':
> > 1, 'testlib': 1, 'gdb': 1, 'pci': 1, 'docs': 1, 'tarch': 1, 'x86 cpuid':
> 1,
> > 'testers': 1, 'debug': 1, 'util/regress': 1, 'branch predictor': 1,
> > 'devices': 1, 'arch/x86': 1, 'kmi': 1, 'x86 isa': 1, 'cpu. arch': 1,
> 'sym':
> > 1, 'rcs scripts': 1, 'regress': 1, 'ide': 1, 'cache recorder': 1, 'o3
> iew':
> > 1, 'unittest': 1, 'pseudo inst': 1, 'build opts': 1, 'pl011': 1, 'hmc':
> 1,
> > 'decoder': 1, 'learning-gem5': 1, 'o3cpu': 1, 'sconstruct': 1,
> 'cpu-kvm':
> > 1, 'ruby sequencer': 1, 'ext lib': 1, 'mem-garnet': 1, 'arch-generic': 1,
> > 'options': 1}
> > >
> > > Matching against the current tags existing in maintainers (not
> including
> > dev-arm, which shall be added soon), this is the list:
> > >
> > > {'mem': 575, 'cpu': 367, 'arch-arm': 251, 'sim': 214, 'stats':
> 194,
> > 'mem-cache': 193, 'dev': 192, 'base': 187, 'tests': 144, 'scons': 142,
> > 'misc': 113, 'arch': 93, 'python': 73, 'util': 70, 'configs': 67,
> > 'mem-ruby': 63, 'sim-se': 51, 'ext': 48, 'gpu-compute': 46, 'arch-riscv':
> > 43, 'cpu-o3': 23, 'arch-x86': 19, 'system-arm': 12, 'arch-mips': 3,
> > 'arch-power': 3, 'cpu-minor': 3, 'arch-hsail': 2, 'arch-sparc': 2,
> > 'arch-alpha': 1, 'cpu-kvm': 1, 'mem-garnet': 1, 'learning-gem5': 1}
> > >
> > > Therefore these gem5 tags have not been used in a long time:
> > >
> > > {'dev-virtio', 'system-alpha', 'sim-power', 'system',
> 'cpu-simple'}
> > >
> > > 'dev-virtio' has been used as 'virtio' (3)
> > > 'system-alpha' has been used as 'alpha' (21) and 'arch-alpha' (1)
> > >
> > > 'power' (26) has both been used as 'sim-power' and 'arch-power', which
> > are completely different
> > >
> > > 'system' and 'cpu-simple' seem to have been substituted by other tags
> > >
> > > =============================================
> > >
> > >
> > > Further analysis:
> > >
> > > Except for arm and riscv, which had tied results, there seems to exist
> a
> > preference to not include "arch-" in ISA specific commits, so I think
> they
> > should all be replaced, i.e., 'arch-x86' becomes 'x86', 'arch-hsail'
> > becomes 'hsail' and so on, but I also agree with the counterargument that
> > these tags reflect the path to the dir, and therefore 'arch-' must be
> > present.
> > >
> > > The same applies to "mem-ruby", which is constantly used as "ruby".
> > > Maybe 'learning-gem5' should be move to 'misc' due to their low
> > frequency. Also, move 'mem-garnet' to 'misc', since although it belongs
> to
> > 'mem' I do not know if the maintainer is familiar with it.
> > > The usage of 'misc' has been fair, overall, although sometimes a new
> tag
> > 'style' has been used instead, and other times other tags should have
> been
> > applied instead.
> > > 'dev-virtio' is very rare, so it could be incorporated by 'dev'.
> > > 'cpu-minor' is very rare, so it could be incorporated by 'cpu'
> > >
> > > There is no maintainer for 'cpu-o3'. Does it really need to be apart
> > from 'cpu'?
> > >
> > > I do not know what to think of the 'system', 'system-alpha', and
> > 'system-arm' tags.
> > > =============================================
> > >
> > >
> > > Given all that information, I propose remodeling the list of tags. This
> > is what I am thinking:
> > >
> > > [alpha, arch, arm, base, configs, cpu, cpu-o3, dev, dev-arm, ext,
> > gpu-compute, hsail, kvm, learning-gem5, mem, mem-cache, mips, misc,
> power,
> > python, riscv, ruby, scons, sim, sim-se, sim-power, slicc, sparc, stats,
> > system, system-arm, systemc, tests, util, x86]
> > >
> > > Of course, this is mostly from a statistical point of view, so it may
> > not reflect the knowledge nor will of the maintainers, so I ask you: what
> > do you think?
> > >
> > >
> > > Regards,
> > > Daniel
> > > _______________________________________________
> > > gem5-dev mailing list
> > > [email protected]
> > > http://m5sim.org/mailman/listinfo/gem5-dev
> > >
> > IMPORTANT NOTICE: The contents of this email and any attachments are
> > confidential and may also be privileged. If you are not the intended
> > recipient, please notify the sender immediately and do not disclose the
> > contents to any other person, use it for any purpose, or store or copy
> the
> > information in any medium. Thank you.
> > _______________________________________________
> > gem5-dev mailing list
> > [email protected]
> > http://m5sim.org/mailman/listinfo/gem5-dev
> > _______________________________________________
> > gem5-dev mailing list
> > [email protected]
> > http://m5sim.org/mailman/listinfo/gem5-dev
> > _______________________________________________
> > gem5-dev mailing list
> > [email protected]
> > http://m5sim.org/mailman/listinfo/gem5-dev
> _______________________________________________
> gem5-dev mailing list
> [email protected]
> http://m5sim.org/mailman/listinfo/gem5-dev
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
