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

hotfix type issue from long searches #1003

Merged
merged 1 commit into from
Feb 12, 2024
Merged

Conversation

theosanderson
Copy link
Member

@theosanderson theosanderson commented Feb 12, 2024

Fabian made a suggestion to do this with Astro's formData function, which makes perfect sense, but I guess I was misinterpreting what it returned. For now reverting to known working state.

@corneliusroemer
Copy link
Contributor

corneliusroemer commented Feb 12, 2024

Woops - how did this get past our CI? Lack of having rebased on main - so some changes between branch-off and current main?

@theosanderson
Copy link
Member Author

I saw a "check website formatting" CI fail, so I reformatted and merged. But actually the check website formatting encompasses type checking too.

Copy link
Contributor

@corneliusroemer corneliusroemer left a comment

Choose a reason for hiding this comment

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

Merge when it passes CI 😝

@corneliusroemer
Copy link
Contributor

But it didn't error in the PR - only when it was rebased onto main, right? Maybe we should always do that as last check before merging - rebase on main and force push to check it applies cleanly.

@theosanderson
Copy link
Member Author

no, this one wasn't about merging. (And IMO making everyone rebase wouldn't be worth the time-cost for this stage of the project)

@corneliusroemer
Copy link
Contributor

You're right, the last tick was just the cache cleanup and not any real checks

image

Rebase takes like 10 seconds with fork git gui 🙈

@theosanderson
Copy link
Member Author

theosanderson commented Feb 12, 2024

I don't have fork git GUI :) (and would prefer to have flexibility to work in environments without)

@theosanderson theosanderson merged commit 8291962 into main Feb 12, 2024
12 checks passed
@theosanderson theosanderson deleted the theosanderson-patch-4 branch February 12, 2024 17: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.

2 participants