Skip to content

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Nov 29, 2022

Prefer stable /timestamp_to_event endpoint first, then fallback to the unstable MSC3030 prefixed version.

MSC3030 has been merged into the spec.

Stabilized in Synapse via matrix-org/synapse#14471

Dev notes

Add console color codes

enum AnsiColorCode {
    Red = 31,
    Green = 32,
    Yellow = 33,
}
// Add color to text in the terminal output
function decorateStringWithAnsiColor(inputString: string, decorationColor: AnsiColorCode): string {
    return `\x1b[${decorationColor}m${inputString}\x1b[0m`;
}

Todo

  • Create follow-up PR to add ANSI color codes back to test assertions

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

This change is marked as an internal change (Task), so will not be included in the changelog.

@MadLittleMods MadLittleMods marked this pull request as ready for review November 29, 2022 22:51
@MadLittleMods MadLittleMods requested a review from a team as a code owner November 29, 2022 22:51
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

With my web hat on, this does need a test to ensure the correct endpoints are called in fallback scenarios (if we're going that route). Otherwise, the unstable-stable flag or version checks need to be tested.

Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

Overall this is looking better - thanks for adding the test! Just some points on the code itself and this should be fine to land.

Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

@MadLittleMods thank you for all your work on this. With the little changes I added, it looks great.

Co-authored-by: Eric Eastwood <erice@element.io>
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

otherwise lgtm - thanks for taking a look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Tasks for the team like planning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants