I must say, that review process is not a straightforward as it can be. I have also some PR and I can see similar problems as you have with your PR. There were helpful review comments but after some time there weren't any further reactions (positive or negative). I understand that for open source projects there are some delays quite often but my observation is that there are three types of PR (with some simplification): 1) simple changes - no problem with a review 2) changes (sometimes quite complex) backboned by some "high level" developers (Oracle and some other people) - review process & merging is quite straightforward 3) some complex or not trivial changes without "support" - from time to time there are problems with review & merging
Maybe now it is time to talk how we (developer,reviewers) could improve point 3. On Tue, Jul 6, 2021 at 1:44 AM Eric Bresie <[email protected]> wrote: > > I know before sending this, that it will be perceived as yet another long > email including a lot of extra and/or editorial details…I’ve done long and > I’ve done short without much luck either way. > > It was suggested before taking on a larger task (i.e. Python), that I try a > smaller task. So this was my first attempt (for what I believed to be a > smaller task) working a Netbeans ticket and submitting a PR. I am by no > means an expert on all things Netbeans internals (and GitHub activities) > which is why I’ve been asking so many questions and looking for mentoring > on such things. > > The latest cleaned up version has resulted in a small fix of 3 file changes > visible in the PR. Most of the discussions are resolved or awaiting > acceptance of most recent changes. > > While not a wiki page, I’ve documented it in mailing list threads, > NETBEANS-189 JIRA ticket, PR-2820 discussions and have discussed it on > Stack Netbeans general channel. > > If the requirement for a fix is to document it on a wiki or document line > by line, I would be interested to see other such actions on recent changes. > > I’ve responded multiple times on many threads (1), (2), (3), (4), (5), > including responses to your most recent (5) in which I summarized again, > showed how it was tested, and included a video showing the fix in action. > > I’ve asked questions receiving helpful responses; I’ve asked question with > no responses; I’ve asked lengthy and specific questions (assuming more > specifics were need) to focus the details with no responses; I’ve asked > short one…no response. I’m really not sure how else to communicate on such > things. > > I know everyone has other work to and acknowledge not everyone has the > expertise, inclinations, or time to do so which is why I’ve been patient > waiting from inclusion for multiple releases raising the questions multiple > times. > > I had some bad changes (i.e. initial attempts for “SQL Hints” which were > removed for future work later, had some commit auto build issues [due to > missed bad change removal], attempted “connection less” refactoring without > much luck [this is a more complex architectural change out of scope of > this] but create a separate ticket to address it later, etc.) which I’ve > removed, updated and/or learned from. > > Here is yet another attempt to explain the problem and the PR changes. > > Problem is when no connection is available, it’s impossible to autocorrect > with DBs, tables, and fields identifiers. So no auto-completion of DB > based is possible. However it goes further in that even basic SQL > auto-completion is not possible. As written rather than handle basic SQL > completion, it opens a dialog asking to establish a connection which is > fine but still prevents basic SQL auto-completion. > > Change involve: > - Checking in specific applicable locations in the Sql Editor project when > a “quoter” is null and allows it to recover and progress further. The > quoter is usually null because at the time there is no connection used in > determining identifiers and how to quote them. Identifiers and quoters used > during autocompletion are dependent on the DB (connection) involved have to > account for different DB vendors implementation of identifiers and quoting > of identifiers (i.e. some DB allow quotes, allow spaces, single > quote/double quotes, etc.). If the quoter is not available (i.e. no > connection), prior to the fix, this prevented any sql auto completion (i.e. > can’t even auto complete the start of an sql expression). With the current > fixes, this is possible. > - Additional comments were also added in applicable locations to help. > > Should the PR just rejected, create a new PR with only the most recent > changes and start over? I’m a little hesitant as there is a lot of (as has > been pointed out) what I view as valuable details and lessons learned. > > So….what now? > > References: > (1) Initial question on issue and how to proceed > https://lists.apache.org/thread.html/r3d81870ee2dda606e2a2ea2d23b48efca3dc0ed2c0e4976316e41a3b%40%3Cdev.netbeans.apache.org%3E > > (2) With requests and response including attempt (and removal) of > refactoring > https://lists.apache.org/thread.html/r212d97b370759b0cd32c8818915e71d4501b6d9317229e84a6ad707a%40%3Cdev.netbeans.apache.org%3E > > (3) Further updates and adding/removal of hints and annotations and build > issues > https://lists.apache.org/thread.html/rbff4e502afa275c2b78023d366e6e41bd0345fd7a1ab513e7f0561a1%40%3Cdev.netbeans.apache.org%3E > > (4) Nearing completion with discussion on refactoring > connection/connectionless and links to new tickets raise in the process for > future work > https://lists.apache.org/thread.html/rf7e71e41291a33c286a55875e501fd849a2932373b93eccbaa55b30c%40%3Cdev.netbeans.apache.org%3E > > (5) Request and response > https://lists.apache.org/thread.html/r84872aa56881043a18ff2904bacee046167f2fafe69dedc97545a772%40%3Cdev.netbeans.apache.org%3E > > > (6) Latests thread showing summary details including test details and a > link to video showing the fix in action > https://lists.apache.org/thread.html/r88c0ee9aaba1c337ff306bf1349325c05b269b22eca43e717653a618%40%3Cdev.netbeans.apache.org%3E > > On Sun, Jul 4, 2021 at 8:23 PM Geertjan Wielenga < > [email protected]> wrote: > > > I’ve asked you this in another thread but got no response — you have done > > so much work and rework on this pull request that it is hard to tell the > > wood from the trees. Probably almost nobody can review this PR anymore. > > > > Are you able to explain and account for every statement you’re proposing > > adding to the repository? > > > > If so, can you create a Wiki page or something somewhere with step by step > > instructions so someone can see what exactly your pull request consists of > > (explain from scratch what we’re trying to achieve with this pr and > > precisely what code needs to be put where in our repo to achieve that)? > > > > If not, i.e., if you cannot account for each line of code that you’re > > proposing to add, if you have hacked things together here and there, if > > there are parts that you don’t completely understand or can’t explain — > > then you probably don’t want this code in our repo yourself, because then > > there’ll be a lot of problems that someone else (who? you?) will be fixing > > to potentially even get NetBeans even to start. > > > > Gj > > > > > > On Mon, 5 Jul 2021 at 02:20, Eric Bresie <[email protected]> wrote: > > > >> https://github.com/apache/netbeans/pull/2820 > >> > >> On Sun, Jul 4, 2021 at 6:50 PM Geertjan Wielenga > >> <[email protected]> wrote: > >> > >>> Can you point to the PR you’re referring to? > >>> > >>> Gj > >>> > >>> On Mon, 5 Jul 2021 at 01:12, Eric Bresie <[email protected]> wrote: > >>> > >>> > How’s does one get something approved? > >>> > > >>> > The last change requested was against old out of date files (changes > >>> were > >>> > made since then). I’ve seen no further comments, reviews and/or > >>> approval > >>> > for close to 3 months now beyond my own comment (with new ticket to > >>> address > >>> > the major architectural changes requested). > >>> > > >>> > Eric > >>> > > >>> > On Sat, Jul 3, 2021 at 10:32 AM Geertjan Wielenga > >>> > <[email protected]> wrote: > >>> > > >>> > > Once a PR has been approved and merged, let’s think about setting > >>> tags > >>> > and > >>> > > so on for it at that stage. > >>> > > > >>> > > Gj > >>> > > > >>> > > > >>> > > On Sat, 3 Jul 2021 at 17:16, Eric Bresie <[email protected]> wrote: > >>> > > > >>> > > > There is a JIRA ticket (NETBEANS-189 > >>> > > > <https://issues.apache.org/jira/browse/NETBEANS-189>) which allows > >>> > > labels > >>> > > > and target releases but github / PRs are different system(s). > >>> > > > I assume only official committers on the apache/netbeans with the > >>> > > > applicable roles and/or permissions are allowed to do so. > >>> > > > > >>> > > > So then am I correct in saying I will need an "official committer" > >>> to > >>> > do > >>> > > > so? > >>> > > > > >>> > > > Eric Bresie > >>> > > > [email protected] > >>> > > > > >>> > > > > >>> > > > On Fri, Jul 2, 2021 at 1:41 PM Piotr Hoppe <[email protected]> > >>> > wrote: > >>> > > > > >>> > > > > Did you check a JIRA ticket? > >>> > > > > > >>> > > > > Piotr > >>> > > > > > >>> > > > > W dniu 2021-07-02 o 19:42, Eric Bresie pisze: > >>> > > > > > How does one flag the milestone and/or labels of a PR? Are > >>> there > >>> > > > specific > >>> > > > > > steps or permissions needed? On a PR I'm working on I don't > >>> see > >>> > any > >>> > > > way > >>> > > > > to > >>> > > > > > do so or even if I'm allowed to. > >>> > > > > > > >>> > > > > > Eric Bresie > >>> > > > > > [email protected] > >>> > > > > > > >>> > > > > > > >>> > > > > > On Fri, Jul 2, 2021 at 11:03 AM Eric Barboni <[email protected] > >>> > > >>> > > wrote: > >>> > > > > > > >>> > > > > >> Just a reminder to all commiters please flag the milestone to > >>> 12.5 > >>> > > on > >>> > > > > merge > >>> > > > > >> it's ease to construct Release Note. > >>> > > > > >> > >>> > > > > >> > >>> > > > > > >>> > > > > > >>> > > > > > >>> --------------------------------------------------------------------- > >>> > > > > To unsubscribe, e-mail: [email protected] > >>> > > > > For additional commands, e-mail: [email protected] > >>> > > > > > >>> > > > > For further information about the NetBeans mailing lists, visit: > >>> > > > > > >>> https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists > >>> > > > > > >>> > > > > > >>> > > > > > >>> > > > > > >>> > > > > >>> > > > >>> > -- > >>> > Eric Bresie > >>> > [email protected] > >>> > > >>> > >> -- > >> Eric Bresie > >> [email protected] > >> > > -- > Eric Bresie > [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected] For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
