RE >I think this small change would significantly improve the confidence in code reviewers during review and also make it easier to track down issues if they arise at a later stage.
So, expanding... any time you see a change to airflow/api_fastapi, you are probably making changes to "the interface". Particularly if you see changes to `v1-generated.yaml`. We have multiple APIs within that folder -- some of which are public and others which are internal. And then of course there are various bits of airflow code that are public interface and others that are not. Perhaps instead of a checkbox we could auto-add a label that indicates which API is being touched, based on which subfolder of api_fastapi. This might be a better way to help meet your aim of helping with tracking down changes, that isn't reliant on humans checking a box (and doesn't add the noise) that people will likely generally ignore. On Fri, Feb 21, 2025 at 11:07 AM Daniel Standish < daniel.stand...@astronomer.io> wrote: > It feels more like a failure of CI / testing than a failure of PR > description. > > Is an author supposed to exercise all APIs for every "public interface" > change? > > On Fri, Feb 21, 2025 at 8:52 AM Ankit Chaurasia <sunank...@gmail.com> > wrote: > >> Hi Amogh, >> >> Great idea! I agree with adding a checkbox for public interface changes in >> the PR template. Additionally, I suggest we add a brief section where >> contributors can note any potential side effects or impacts if known. This >> extra step could help reviewers catch issues early. >> >> Additionally, the API label was not automatically added to this PR. >> >> Regards, >> *Ankit Chaurasia* >> HomePage <https://ankitchaurasia.info/> | LinkedIn >> <https://www.linkedin.com/in/sunank200/> >> >> >> >> >> >> >> On Fri, Feb 21, 2025 at 12:10 PM Amogh Desai <amoghde...@apache.org> >> wrote: >> >> > Hello Everyone, >> > >> > I have noticed an increased number of PRs introducing changes to the >> public >> > interfaces of Airflow (UI / API / CLI) do not provide sufficient >> > information / evidence about their working, either in form of >> screenshots >> > (for UI mainly) and/or API responses. >> > >> > For example, a recent PR merge: >> > https://github.com/apache/airflow/pull/46666, >> > while not showing any apparent side effects during code review, ended up >> > breaking a ton of UI and API and as a result, follow up issues and PRs >> had >> > to be created to fix it. >> > >> > To minimize such occurrences, I propose we slightly modify the PR >> template >> > to include a checkbox that could have content along the lines of: >> > *- My PR introduces a public interface change (UI/API), and I have added >> > screenshots and/or API responses before and after my change.* >> > >> > I think this small change would significantly improve the confidence in >> > code reviewers during review and also make it easier to track down >> issues >> > if they arise at a later stage. >> > >> > I personally believe that this would enhance our review process and >> reduce >> > such occurrences. >> > Interested to hear your opinions and other suggestions that you may >> have. >> > >> > Thanks, >> > Amogh >> > >> >
