Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-17322: [Docs] Documenting issue lifecycle for bugs and feature requests #13781

Merged
merged 11 commits into from
Aug 24, 2022

Conversation

toddfarmer
Copy link
Contributor

@toddfarmer toddfarmer commented Aug 2, 2022

These changes aim to document the issue lifecycle - how to create new issues (largely already existed) as well as how to engage with and understand issue state later on. This also serves to document proposed processes:

  1. Assigned issues idle more than 90 days may be unassigned (discussed and approved on ML)
  2. Unassigned issues in "In Progress" status should be set to "Open"
  3. Establishing consistent usage of Status and Resolution fields.

the issue and starting progress. Issues in this state should be unassigned.
* **In progress** - At the time a contributor self-assigns an issue, the status
should be set to In progress by clicking the **Start progress** button. All
issues in this status should have an assignee - unassigned issues will be
Copy link
Contributor Author

@toddfarmer toddfarmer Aug 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: This details a newly proposed project policy - that any issues that have a status of "In Progress" but lack an assignee should be reverted to "Open" status. This prevents issues from being misrepresented as "In Progress" when there is nobody actually working the issue. This is likely to be automated, and while manual inspection of an issue in such a state may indicate somebody is likely working the issue (e.g., adding comments, etc.) but neglected (or cannot) self-assign, that may not be recognized in an automated process. Any automation should include a comment when changing the status from "In Progress" to "Open" for this reason, guiding any affected users on how to self-assign if appropriate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

* Implemented
* Done

* **Closed** - Another terminal status, Closed indicates the issue is complete,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: Resolution codes have not been consistently applied historically. See this analysis for more details, and to comment on proposal to standardized and clean existing resolution code data. The resolution codes listed here eliminate several codes that have been previously used, and aims to standardize usage for clarity.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the problem here is there are too many possible choices with no one being obviously better than the others in some situations :-)

@toddfarmer
Copy link
Contributor Author

The attached .zip file contains the complete HTML page to help with review:
Bug reports and feature requests — Apache Arrow v9.0.0.zip

Copy link
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @toddfarmer ! Minor comment from me

`create an ASF Jira account here <https://issues.apache.org/jira/secure/Signup!default.jspa>`_.
Self-assigning issues to work requires
the "Contributor" permission. This permission is set by project maintainers,
and may be requested by emailing the dev mailing list.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add the full email address? As a new user reporting an issue it might not be clear where to find the dev mailing list.

Suggested change
and may be requested by emailing the dev mailing list.
and may be requested by emailing the `dev@arrow.apache.org <https://mail-archives.apache.org/mod_mbox/arrow-dev/>`_ mailing list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! I changed this to point to the mailing list section of the communication page, just because I'm not sure of the mechanics of a non-subscriber emailing the dev@ list (will it go through? will they see any reply?).

@kou
Copy link
Member

kou commented Aug 5, 2022

Could you open a JIRA issue for this?
This is not a minor fix: https://github.com/apache/arrow/blob/master/CONTRIBUTING.md#minor-fixes

  • Grammar, usage and spelling fixes that affect no more than 2 files
  • Documentation updates affecting no more than 2 files and not more than 500 words.

@toddfarmer toddfarmer changed the title MINOR: Documenting issue lifecycle for bugs and feature requests ARROW-17322: [Docs] Documenting issue lifecycle for bugs and feature requests Aug 5, 2022
@github-actions
Copy link

github-actions bot commented Aug 5, 2022

@toddfarmer
Copy link
Contributor Author

Could you open a JIRA issue for this? This is not a minor fix:

Happy to, thanks for the prompt: ARROW-17322.

@toddfarmer toddfarmer force-pushed the toddfarmer/document-issue-lifecycle branch from dd6d2e0 to fe5de40 Compare August 5, 2022 16:17
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

toddfarmer and others added 3 commits August 9, 2022 10:47
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge this after a few days to wait for comments from others.

closed, including during review of pull requests.
* **Resolved** - This is a terminal status indicating action has been taken
on the issue, which is now considered completed. Issues in a resolved status
may have the following resolution codes set:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should really always use "Fixed" for consistency, IMHO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that true for non-defect issues (e.g., feature requests)? If a status of "Resolved" should map to a resolution of "Fixed" 100% of the time, that's easy enough to automate (in addition to document).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is. The merge script does that. Besides, I don't think "Implemented" or "Done" would convey any useful subtlety compared to "Fixed".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not entirely sure what to make of ARROW-16790 in this context. The PR was merged, but the resolution is "Resolved" instead of "Fixed".

I've updated this page to direct usage of "Fixed" when resolving an issue. I scanned this list for examples where that's not set, and it seems to mostly be manually-set resolutions, either using a status of "Resolved" when it should be more properly be "Closed" (e.g., "Information Provided" or "Won't Fix" resolutions), or because the issue was resolved by work done on a different issue (and the PR referenced the other issue, not the one with the incorrect resolution).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ARROW-16790 should ideally be marked "Fixed". But it seems that would require reopening the issue... let's not bother with this one perhaps.

Co-authored-by: Antoine Pitrou <[email protected]>
Copy link
Member

@AlenkaF AlenkaF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small comments regarding the self-assignment of an issue.

`create an ASF Jira account here <https://issues.apache.org/jira/secure/Signup!default.jspa>`_.
Self-assigning issues to work requires
the "Contributor" permission. This permission is set by project maintainers,
and may be requested by emailing the :ref:`development mailing list <mailing_list>`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need for a contributor permission to self-assign Jira issues anymore, if I understand correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to reflect this change - thanks!

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates @toddfarmer !

@pitrou pitrou merged commit d53bce2 into apache:master Aug 24, 2022
@ursabot
Copy link

ursabot commented Aug 24, 2022

Benchmark runs are scheduled for baseline = c90d07f and contender = d53bce2. d53bce2 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️25.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️1.11% ⬆️0.0%] test-mac-arm
[Failed ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Failed ⬇️0.0% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] d53bce20 ec2-t3-xlarge-us-east-2
[Failed] d53bce20 test-mac-arm
[Failed] d53bce20 ursa-i9-9960x
[Failed] d53bce20 ursa-thinkcentre-m75q
[Finished] c90d07f7 ec2-t3-xlarge-us-east-2
[Failed] c90d07f7 test-mac-arm
[Failed] c90d07f7 ursa-i9-9960x
[Failed] c90d07f7 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@ursabot
Copy link

ursabot commented Aug 24, 2022

['Python', 'R'] benchmarks have high level of regressions.
ec2-t3-xlarge-us-east-2

pitrou added a commit to pitrou/arrow that referenced this pull request Aug 24, 2022
PR apache#13781 moved back the testing submodule to an old changeset by mistake,
which subsequently broke multiple CI builds.

(added a dummy C++ change to trigger CI)
pitrou added a commit that referenced this pull request Aug 24, 2022
PR #13781 moved back the testing submodule to an old changeset by mistake, which subsequently broke multiple CI builds.

(added a dummy C++ change to trigger CI)

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
anjakefala pushed a commit to anjakefala/arrow that referenced this pull request Aug 31, 2022
…requests (apache#13781)

These changes aim to document the issue lifecycle - how to create new issues (largely already existed) as well as how to engage with and understand issue state later on. This also serves to document proposed processes:

1. Assigned issues idle more than 90 days may be unassigned (discussed and approved on ML)
2. Unassigned issues in "In Progress" status should be set to "Open"
3. Establishing consistent usage of Status and Resolution fields.

Authored-by: Todd Farmer <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
anjakefala pushed a commit to anjakefala/arrow that referenced this pull request Aug 31, 2022
PR apache#13781 moved back the testing submodule to an old changeset by mistake, which subsequently broke multiple CI builds.

(added a dummy C++ change to trigger CI)

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
zagto pushed a commit to zagto/arrow that referenced this pull request Oct 7, 2022
…requests (apache#13781)

These changes aim to document the issue lifecycle - how to create new issues (largely already existed) as well as how to engage with and understand issue state later on. This also serves to document proposed processes:

1. Assigned issues idle more than 90 days may be unassigned (discussed and approved on ML)
2. Unassigned issues in "In Progress" status should be set to "Open"
3. Establishing consistent usage of Status and Resolution fields.

Authored-by: Todd Farmer <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
zagto pushed a commit to zagto/arrow that referenced this pull request Oct 7, 2022
PR apache#13781 moved back the testing submodule to an old changeset by mistake, which subsequently broke multiple CI builds.

(added a dummy C++ change to trigger CI)

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
fatemehp pushed a commit to fatemehp/arrow that referenced this pull request Oct 17, 2022
PR apache#13781 moved back the testing submodule to an old changeset by mistake, which subsequently broke multiple CI builds.

(added a dummy C++ change to trigger CI)

Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants