Skip to content

Comments

Duplicate Due column Fix#18144

Merged
mikehardy merged 1 commit intoankidroid:mainfrom
dorrin-sot:fix-bugs/17892-Duplicate-Due-column
Apr 5, 2025
Merged

Duplicate Due column Fix#18144
mikehardy merged 1 commit intoankidroid:mainfrom
dorrin-sot:fix-bugs/17892-Duplicate-Due-column

Conversation

@dorrin-sot
Copy link
Contributor

@dorrin-sot dorrin-sot commented Mar 21, 2025

Purpose / Description

The columns sometimes were not distinct in CardBrowser.

Fixes

Approach

I modified BrowserColumnCollection::load to return only distinct columns.

How Has This Been Tested?

I wrote a unit test to override the CardBrowser columns in SharedPreferences so it has duplicate values and checked its distinctness afterward.

Checklist

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

@dorrin-sot dorrin-sot changed the title Fix bugs/17892 duplicate due column Duplicate Due column Fix Mar 22, 2025
Copy link
Member

@Arthur-Milchior Arthur-Milchior left a comment

Choose a reason for hiding this comment

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

Pretty nice commit, thanks a lot.
My only regret is that it was not clear why it was indeed the right commit. In particular, I didn't know that it ensured that the preference would be corrected the next time the user change it as a side effect of your commit.

So, I approve the code and the test, still, I'd really want that your commit message, now and in the future, be more descriptive please. That you help reviewers understand what you did, why it works, what impact it has, without us having to look in details at the code you didn't touch

@Arthur-Milchior Arthur-Milchior added the Needs Second Approval Has one approval, one more approval to merge label Mar 23, 2025
Copy link
Member

@david-allison david-allison left a comment

Choose a reason for hiding this comment

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

Looks great, cheers!

@david-allison david-allison added Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) Needs reviewer reply Waiting for a reply from another reviewer and removed Needs Second Approval Has one approval, one more approval to merge labels Mar 25, 2025
Copy link
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@mikehardy mikehardy added squash-merge A squash & force push is required. The PR author may do this to speed up the merge process. and removed Needs reviewer reply Waiting for a reply from another reviewer labels Apr 5, 2025
- Added Unit test for Fix ankidroid#17892 Duplicate Due column in Card Browser
@mikehardy mikehardy force-pushed the fix-bugs/17892-Duplicate-Due-column branch from 55b0c16 to 893d8eb Compare April 5, 2025 17:43
@mikehardy mikehardy removed the squash-merge A squash & force push is required. The PR author may do this to speed up the merge process. label Apr 5, 2025
@mikehardy
Copy link
Member

squashed locally and re-pushed so I can use merge queue

@mikehardy mikehardy enabled auto-merge April 5, 2025 17:44
@mikehardy mikehardy added this pull request to the merge queue Apr 5, 2025
Merged via the queue into ankidroid:main with commit a33be05 Apr 5, 2025
9 checks passed
@github-actions github-actions bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Apr 5, 2025
@github-actions github-actions bot added this to the 2.21 release milestone Apr 5, 2025
@dorrin-sot dorrin-sot deleted the fix-bugs/17892-Duplicate-Due-column branch April 5, 2025 19:31
@david-allison
Copy link
Member

Hi there @dorrin-sot! This is the OpenCollective Notice for PRs merged from 2025-04-01 through 2025-04-30

If you are interested in compensation for this work, the process with details is here: https://github.com/ankidroid/Anki-Android/wiki/OpenCollective-Payment-Process#how-to-get-paid

We only post one comment per person per month to avoid spamming you, regardless of the number of PRs merged, but this note applies to all PRs merged for this month

Please understand that our monthly budget is never guaranteed to cover all claims - the cap on payments-per-person may be lower, but we try to make our process as fair and transparent as possible, we just need your understanding.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicate Due column

4 participants