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

feat: Add --help, introduce "any" reason, explain filter when no notifications found #196

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
coverage*/
lib/
node_modules/
.aider*
Copy link
Owner

Choose a reason for hiding this comment

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

[Tooling]

Suggested change
.aider*

Which reminds me, thanks - JoshuaKGoldberg/dot-com#337

2 changes: 2 additions & 0 deletions src/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ describe("pruneGitHubNotificationsCLI", () => {
reason: new Set(["abc", "def"]),
title: [/abc.+def/],
},
logFilterWhenEmpty: true,
});
expect(mockRunInWatch).not.toHaveBeenCalled();
});
Expand All @@ -63,6 +64,7 @@ describe("pruneGitHubNotificationsCLI", () => {
reason: new Set(["abc", "def"]),
title: [/abc.+def/],
},
logFilterWhenEmpty: false,
});
expect(mockRunInWatch).toHaveBeenCalledWith(expect.any(Function), 10);
});
Expand Down
30 changes: 30 additions & 0 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,27 @@ import * as z from "zod";
import { pruneGitHubNotifications } from "./pruneGitHubNotifications.js";
import { runInWatch } from "./runInWatch.js";

function printHelp() {
console.log(`
prune-github-notifications

Prunes GitHub notifications you don't care about, such as automated dependency bumps. 🧹

Options:
--auth GitHub auth token (default: process.env.GH_TOKEN)
--bandwidth Maximum parallel requests (default: 6)
--reason Notification reason(s) to filter (default: "subscribed")
--title Title regex pattern(s) to filter (default: dependency updates)
--watch Seconds interval to continuously run (default: 0)
--help Show this help message

Examples:
npx prune-github-notifications
npx prune-github-notifications --reason subscribed --title "^chore.+ update .+ to"
npx prune-github-notifications --watch 10
`);
}

const schema = z.object({
bandwidth: z.coerce.number().optional(),
reason: z
Expand All @@ -28,6 +49,9 @@ export async function pruneGitHubNotificationsCLI(args: string[]) {
bandwidth: {
type: "string",
},
help: {
type: "boolean",
},
reason: {
multiple: true,
type: "string",
Expand All @@ -43,6 +67,11 @@ export async function pruneGitHubNotificationsCLI(args: string[]) {
tokens: true,
});

if (values.help) {
printHelp();
return;
}

const { bandwidth, reason, title, watch } = schema.parse(values);

const action = async () =>
Expand All @@ -52,6 +81,7 @@ export async function pruneGitHubNotificationsCLI(args: string[]) {
reason,
title,
},
logFilterWhenEmpty: !watch,
});

await (watch ? runInWatch(action, watch) : action());
Expand Down
2 changes: 1 addition & 1 deletion src/createThreadFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ export interface FilterableThreadSubject {

export function createThreadFilter({ reason, title }: FilterOptions) {
return (thread: FilterableThread) =>
reason.has(thread.reason) &&
(reason.has("any") || reason.has(thread.reason)) &&
title.some((tester) => tester.test(thread.subject.title));
}
23 changes: 23 additions & 0 deletions src/pruneGitHubNotifications.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,27 @@ describe("pruneGitHubNotifications", () => {
]
`);
});

it("logs a message when no notifications match the filters", async () => {
Copy link
Owner

Choose a reason for hiding this comment

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

[Testing] If we're going with the feature as-is, I'd request a test for the inverse - that when logFilterWhenEmpty is false, it doesn't log. I.e.:

Suggested change
it("logs a message when no notifications match the filters", async () => {
it("does not log a message when no notifications match the filters and logFilterWhenEmpty is false", async () => { ... });
it("logs a message when no notifications match the filters and logFilterWhenEmpty is true", async () => { ... });

...but, TBD, maybe we'll go with something fancier?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yea we should probably just do a verbose flag or something! I don't know how you feel about the debug module, but I'm also a big fan of that

vi.spyOn(console, "log").mockImplementation(() => {
// Noop
});

mockRequest.mockResolvedValueOnce({
data: [
{
id: "90",
reason: "different",
subject: {
title: "unmatched title",
},
},
],
});

const result = await pruneGitHubNotifications({ logFilterWhenEmpty: true });

expect(result.threads).toEqual([]);
expect(console.log).toHaveBeenCalled();
});
});
24 changes: 19 additions & 5 deletions src/pruneGitHubNotifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export async function pruneGitHubNotifications({
auth,
bandwidth = defaultOptions.bandwidth,
filters,
logFilterWhenEmpty = false,
}: PruneGitHubNotificationsOptions = {}): Promise<PruneGitHubNotificationsResult> {
const octokit = await octokitFromAuth({ auth });

Expand All @@ -26,10 +27,13 @@ export async function pruneGitHubNotifications({
"X-GitHub-Api-Version": "2022-11-28",
},
});
const threadFilter = createThreadFilter({

const filtersWithDefaults = {
reason: filters?.reason ?? defaultOptions.filters.reason,
title: filters?.title ?? defaultOptions.filters.title,
});
};

const threadFilter = createThreadFilter(filtersWithDefaults);

// TODO: Why is the type not being friendly?
const throttle = (throttledQueue as unknown as ThrottledQueue)(
Expand All @@ -38,9 +42,19 @@ export async function pruneGitHubNotifications({
1000,
);

const threads = notifications.data
.filter(threadFilter)
.map((thread) => Number(thread.id));
const matchingThreads = notifications.data.filter(threadFilter);

if (matchingThreads.length === 0) {
if (logFilterWhenEmpty) {
console.log(
"No notifications found matching the filter criteria:",
filtersWithDefaults,
);
}
return { threads: [] };
}

const threads = matchingThreads.map((thread) => Number(thread.id));

await Promise.all(
threads.map(async (thread) => {
Expand Down
6 changes: 6 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ export interface PruneGitHubNotificationsOptions {
auth?: string;
bandwidth?: number;
filters?: Partial<FilterOptions>;

/**
* Controls whether or not to log when no notifications are found. This can
* help new users understand the tool and what filters are being applied.
*/
logFilterWhenEmpty?: boolean;
}

export interface PruneGitHubNotificationsResult {
Expand Down
Loading