Skip to content

Conversation

@efixler
Copy link
Contributor

@efixler efixler commented Feb 21, 2024

Goal

During a recent NewTab incident I commented out code that populated TimeToRead.

This PR just deletes these comments.

I've left other code referencing TimeToRead in place for now and ticketed a deeper removal task for the future.

I don't have a great way to include steps to test here, but this PR should have been a noop -- there are, however, significant changes to GraphQL types that came about as a result of codegen would appreciate 👀 on that.

@efixler efixler requested a review from a team as a code owner February 21, 2024 21:51
Url: any;
};

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes all arrived here as part of the codegen step of the build. I assume that these are derived from some sort of introspection on the graph. They neither look harmless, nor do they seem directly relevant to the intent of this PR; if folks with better understanding of the graph semantics can advise, I'd appreciate. Happy to punt on this PR is this is questionable.

Copy link
Contributor

Choose a reason for hiding this comment

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

These appear to be updates to the graph made by the Pocket team recently. They're all additive changes, too, which means they won't affect anything this proxy relies on.

@efixler efixler merged commit 2cd9d6f into main Feb 26, 2024
@efixler efixler deleted the delete-comments-from-incident branch February 26, 2024 15:16
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.

3 participants