Skip to content

Get previous template version#5230

Merged
BrunoQuaresma merged 17 commits into
mainfrom
bq/previous-version
Dec 6, 2022
Merged

Get previous template version#5230
BrunoQuaresma merged 17 commits into
mainfrom
bq/previous-version

Conversation

@BrunoQuaresma
Copy link
Copy Markdown
Contributor

We need to get the previous template version in the UI, mostly to get the files related to it and show the diff between them on the template version page. so I'm going to add an endpoint for that. In the meantime, I would appreciate your thoughts about the incoming changes like the SQL query and database fake logic.

@BrunoQuaresma BrunoQuaresma requested a review from a team December 1, 2022 14:36
@BrunoQuaresma BrunoQuaresma self-assigned this Dec 1, 2022
@BrunoQuaresma BrunoQuaresma marked this pull request as ready for review December 2, 2022 13:41
Copy link
Copy Markdown
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Didn't fully understand the logic in the fake db but maybe I've misunderstood what we're trying to do there, also had some questions about the query. In general code looks good.

Comment thread coderd/database/queries/templateversions.sql Outdated
Comment thread coderd/database/queries/templateversions.sql
Comment thread coderd/templateversions.go Outdated
Comment thread coderd/database/databasefake/databasefake.go
Bruno Quaresma and others added 2 commits December 2, 2022 11:32
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Copy link
Copy Markdown
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Ok, after your explanations Bruno and looking at this again, query and database fake implementation looks fine (except fake was missing one thing, see suggestion).

Comment thread coderd/database/databasefake/databasefake.go
Comment thread coderd/database/databasefake/databasefake.go
Bruno Quaresma and others added 2 commits December 5, 2022 13:05
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Copy link
Copy Markdown
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

It'd be nice if you could do the remaining errors => xerrors and error block changes, but otherwise looks great. 👍🏻

@BrunoQuaresma BrunoQuaresma enabled auto-merge (squash) December 5, 2022 17:43
@BrunoQuaresma BrunoQuaresma merged commit e17fd0b into main Dec 6, 2022
@BrunoQuaresma BrunoQuaresma deleted the bq/previous-version branch December 6, 2022 14:15
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants