Code gets reviewed by a committer other than the author of the code. Merging can happen by either.
I also agree that the sendgrid dependency should be reverted and made into a plugin instead. Bolke > On 25 Oct 2017, at 11:07, Ash Berlin-Taylor <[email protected]> > wrote: > > 1. PRs generally aren't "approved" in Github if a core contributor looks at > it and is happy > 2. It was merged by criccomini, not the opener > https://github.com/apache/incubator-airflow/commit/7cb818bbacb2a2695282471591a9e323d8efbf5c > > <https://github.com/apache/incubator-airflow/commit/7cb818bbacb2a2695282471591a9e323d8efbf5c> > 3, 4, 5. I agree, and I made the same point > https://issues.apache.org/jira/browse/AIRFLOW-1723 > <https://issues.apache.org/jira/browse/AIRFLOW-1723> > > Either way this is a bug that it always tries to import unconditionally and > fails when the sendgrid mailer isn't used. > >> On 25 Oct 2017, at 09:48, Sumit Maheshwari <[email protected]> wrote: >> >> Sorry, mistakenly sent halfbaked: >> >> So, the concerns are: >> >> 1. Don't see any approver on the PR, neither +1s. >> 2. The PR author and PR merging guy seem to be same, which I think is >> against the general understanding. >> 3. Why sendgrid got special privileges, and why not 100s of other mail >> services? The Same concern was raised by Ash in JIRA as well. >> 4. Why it was not coded as a plugin. >> 5. Why there is a hard dependency to install Sendgrid module if I am not >> using it. >> >> I think this commit needs to be reverted and a new PR should be raised, >> which adds its as a plugin instead of a core feature. >> >> >> >> On Wed, Oct 25, 2017 at 2:12 PM, Sumit Maheshwari <[email protected]> >> wrote: >> >>> When I fetched the latest master, installed it and ran webserver, it >>> failed with this error: >>> >>> *ImportError: No module named sendgrid* >>> >>> On further investigation, I found that it has been introduced as part of >>> this PR <https://github.com/apache/incubator-airflow/pull/2695> and this >>> JIRA <https://issues.apache.org/jira/browse/AIRFLOW-1723>. >>> Now couple of doubts: >>> >>> 1. >>> >>> >>> >>> >>> Thanks, >>> Sumit Maheshwari >>> cell. 9632202950 >>> >>> >
