Yeah, if it's on the client side and you can --no-verify, that more or less
puts it in the warning category. The problem with --no-verify is that it's
an all or nothing thing, so if I know (for instance) that I'll be
systematically breaking some commit rule (importing/changing externally
sourced code, etc), then that big hammer would make me miss if I did
something legitimately wrong at the same time. It's not the end of the
world, but still not ideal.
Also yes, please add systemc and make me the maintainer.
Gabe
On Tue, Oct 22, 2019 at 3:40 PM Jason Lowe-Power <ja...@lowepower.com>
wrote:
> 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 <gabebl...@google.com> 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 <bbr...@ucdavis.edu> 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 <gem5-dev-boun...@gem5.org> on behalf of Daniel
> Carvalho
> > <
> > > oda...@yahoo.com.br>
> > > Sent: Saturday, October 19, 2019 6:38 AM
> > > To: gem5 Developer List <gem5-dev@gem5.org>
> > > 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 <
> > > nikos.nikole...@arm.com> 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
> > > > gem5-dev@gem5.org
> > > > 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
> > > gem5-dev@gem5.org
> > > http://m5sim.org/mailman/listinfo/gem5-dev
> > > _______________________________________________
> > > gem5-dev mailing list
> > > gem5-dev@gem5.org
> > > http://m5sim.org/mailman/listinfo/gem5-dev
> > > _______________________________________________
> > > gem5-dev mailing list
> > > gem5-dev@gem5.org
> > > http://m5sim.org/mailman/listinfo/gem5-dev
> > _______________________________________________
> > gem5-dev mailing list
> > gem5-dev@gem5.org
> > http://m5sim.org/mailman/listinfo/gem5-dev
> _______________________________________________
> gem5-dev mailing list
> gem5-dev@gem5.org
> http://m5sim.org/mailman/listinfo/gem5-dev
_______________________________________________
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev
