And just to be clear, this is just a continuation of the discussion in this thread. This is not in any way a blocker for Ryan's PR, of course.
On Tue, Feb 26, 2019 at 1:44 PM Nick Allen <n...@nickallen.org> wrote: > > We could also enhance > "metron/dev-utilities/release-utils/validate-jira-for-release" so that it > spits out a warning for any JIRAs that are marked "Next + 1", but don't > have a record in the commit history. That would at least be a reminder > that the JIRA needs some follow-up. > > > > > > On Mon, Feb 25, 2019 at 5:25 PM Justin Leet <justinjl...@gmail.com> wrote: > >> Re: Labeling in Jira, I'm fine with having a be "Next + 1" from a release >> management perspective, but I'd still consider at least taking action on >> followup to be the relevant party's responsibility (implementer or >> whatever >> the case may be). We probably should have a more clear way to tag things >> like this, but I don't believe we do right now. If there's not a harder >> dependency than my memory, chances are high it gets >> overlooked/missed/whatever. >> >> On Mon, Feb 25, 2019 at 4:32 PM Otto Fowler <ottobackwa...@gmail.com> >> wrote: >> >> > I really like the idea of architecture.md -> **/architecture.md. >> > >> > We overall do not have javadoc in a lot of areas, and could maybe start >> > working on it as we go and think about asking for it in reviews. >> > We are also missing the Parser Programmer’s Guide, how to add a parser >> to >> > the metron system/install etc and other things. >> > >> > >> > >> > On February 25, 2019 at 15:22:47, Ryan Merriman (merrim...@gmail.com) >> > wrote: >> > >> > I feel like the code itself is pretty well documented. I updated >> existing >> > javadocs and added javadocs to classes that didn't have them before this >> > PR. In my opinion the level of documentation for these classes has >> > increased significantly. >> > >> > On Mon, Feb 25, 2019 at 1:52 PM Michael Miklavcic < >> > michael.miklav...@gmail.com> wrote: >> > >> > > Tentatively agreed on further clarification of what we consider >> in/out of >> > > scope for documentation re: document something that wasn't documented >> > > before. Ryan, can you give a quick summary of what you *have* >> > added/updated >> > > in documentation on this PR vs what you want to leave out? >> > > >> > > My initial concern in punting on docs right now is that part of what >> made >> > > this PR/task more challenging in the first place was not having >> > > documentation. We risk losing context and detail again if we don't do >> > this >> > > immediately. Would it be reasonable to split it up as follows?: >> > > >> > > 1. Additional overarching documentation feels out of scope - make it a >> > > follow on (see comments below). >> > > 2. Adding documentation to our existing README's and java code >> comments >> > > that describe the new/modified functionality should be in scope >> because >> > > it's part of the unit of work. I expect that a developer should be >> able >> > > to >> > > look at the code, tests, comments, and README's and understand how >> this >> > > code functions without having to start from scratch. >> > > >> > > The way we've handled follow-on work before, at least as far as >> feature >> > > branches are concerned, was to create Jiras and link them to the >> > > appropriate discussions for context. Maybe we can take that one step >> > > further and do the release manager a favor by also labeling the >> > > required/requested release on the Jira as a gating factor. This >> follows >> > our >> > > pattern for intermittent test failure reporting, e.g. >> > > >> > > >> > >> > >> https://issues.apache.org/jira/browse/METRON-1946?jql=project%20%3D%20METRON%20AND%20resolution%20%3D%20Unresolved%20AND%20labels%20%3D%20test-failure%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC >> > > . >> > > >> > > I'm also in favor of continuing to document architecture and technical >> > > details as part of the code base as Ryan and Jon have suggested. I >> think >> > we >> > > should have an "architecture.md" in metron root that replaces this - >> > > >> > > >> > >> > >> https://github.com/apache/metron/blob/d7d4fd9afb19e2bd2e66babb7e1514a19eae07d0/README.md#navigating-the-architecture >> > > and covers the broad architecture with links to the appropriate >> modules >> > for >> > > detail. Minimally, it would be nice if we had a simple diagram showing >> > the >> > > basic flow of data in Metron. I think we probably want an updated >> version >> > > of this wiki entry from back in the day - >> > > >> https://cwiki.apache.org/confluence/display/METRON/Metron+Architecture >> > > >> > > Best, >> > > Mike >> > > >> > > >> > > On Mon, Feb 25, 2019 at 7:18 AM Nick Allen <n...@nickallen.org> >> wrote: >> > > >> > > > I don't think we should hold up this work to document something that >> > > wasn't >> > > > previously documented. A follow-on is sufficient. >> > > > >> > > > On Mon, Feb 25, 2019 at 8:50 AM Ryan Merriman <merrim...@gmail.com> >> > > wrote: >> > > > >> > > > > Recently I submitted a PR < >> > https://github.com/apache/metron/pull/1330> >> > >> > > > > that >> > > > > introduces a large number of changes to a critical part of our >> code >> > > base. >> > > > > Reviewers feel like it is significant enough to document at an >> > > > > architectural level (and I agree). There are a couple points I >> would >> > > > like >> > > > > to clarify. >> > > > > >> > > > > Generally architectural documentation lives in the README of the >> > > > > appropriate module. Do we want to continue documenting >> architecture >> > > > here? >> > > > > I think it makes sense because it will be versioned along with the >> > > code. >> > > > > Just wanted to confirm there are no objections to continuing this >> > > > practice. >> > > > > >> > > > > A reviewer suggested we could accept the PR as is and leave the >> > > > > architectural documentation as a follow on. I think this makes >> sense >> > > > > because it can be tedious to maintain a large PR as other smaller >> > > commits >> > > > > are accepted into master. An important requirement is the >> > > documentation >> > > > > follow on must be completed in a timely manner, before the next >> > > release. >> > > > > Are there any objections to doing it this way? >> > > > > >> > > > >> > > >> > >> >