MyDeveloperDay added a comment.
This patch still only considers structs? @krasimir and I are I think saying
that really this should it support `class` in the same way? I mean isn't a
struct and a class almost identical in this context why would we want treat
them differently.
class new_struct struct_name =
{a = 1};
struct new_struct struct_name =
{a = 1};
Sorry to push you on the patch quality (this and your other patch), I'm
personally always a little extra cautious of peoples first patches, especially
where they are adding new functionality.
This is because we don't need drive by patches where people come in drop just
the functionality they require then disappear, I'm not saying you'll do that
but we have to think about the longer term maintainability of any patch, we
have to understand it and it has to give us the capability that we think the
whole community requires. Your use case is valid, but such a change like this
will live forever so ideally we want it to address everything in a consistent
way. (I'm not even convinced by the Struct in BeforeStructInitialization name,
just saying)
As these are your first patches I'm personally likely to have extra levels of
scrutiny. I'm sorry for that, (its self preservation), I want to know you are
willing to be a contributor who comes back to help and maintain your own patch
as well as others.
This is why we always advise people to start by looking at a few bugs from the
bug tracker, nothing says I'm here to stay like fixing other peoples issues and
not your own requirements. Plus you don't have to justify the change when its
fixing an issue or a regression. Once people have established a "reputation"
for being a consistent/semi consistent contributor then getting new
functionality in is always a little easier. (that may not be fair, but its just
advice on how to get stuff in!)
As I look back at my own first patches they took months to get in (and lots of
diffs), I made the the mistake of wanting to add a new feature
("modernize-use-nodiscard" clang-tidy check), I felt frustration at my patch
constantly coming back, but I stuck at it.
Bug fixes can often be rubber stamped in days and that can be a good learning
curve for those wanting to get started with clang, as you can make 2 or 3
commits quite rapidly, which makes you feel good about contributing. It also
builds trust and confidence so when you want to add something more ambitious,
reviewers have more faith and likely cut you a little more slack. (just saying,
some advice on starting)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91949/new/
https://reviews.llvm.org/D91949
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
