Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix(site): set min and max attributes for workspace number parameters#15182

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

toshikish
Copy link
Contributor

Implements#14532.

This PR setsmin andmax attributes for workspace number parameter<input> elements usinginputProps.

Note: When we update MUI to v6 or later, it is better to useslotProps.htmlInput instead.

@cdr-botcdr-botbot added the communityPull Requests and issues created by the community. labelOct 22, 2024
@matifalimatifali requested review froma team andBrunoQuaresma and removed request fora teamOctober 28, 2024 06:19
@matifali
Copy link
Member

Hi@toshikish, thank you for the contribution. Could you rebase your PR onmain and make all the checks in CI pass? We should be able to review the contribution after that.

@toshikishtoshikishforce-pushed theset-min-max-for-workspace-parameters branch from5b747f1 to6c62431CompareOctober 28, 2024 08:51
@toshikish
Copy link
ContributorAuthor

@matifali Hi, thank you for reviewing. I rebased the branch and fixed the lint and unit test errors found so far.

@toshikish
Copy link
ContributorAuthor

The cause of test-e2e-premium failure is thatlicense is empty.
https://github.com/coder/coder/actions/runs/11565755422/job/32199523539?pr=15182#step:11:131

The environment variableCODER_E2E_LICENSE seems substituted through CI secret, but is it set correctly?
https://github.com/toshikish/coder/blob/set-min-max-for-workspace-parameters/.github/workflows/ci.yaml#L616

@matifali
Copy link
Member

matifali commentedOct 29, 2024•
edited
Loading

The cause of test-e2e-premium failure is thatlicense is empty.coder/coder/actions/runs/11565755422/job/32199523539?pr=15182#step:11:131

The environment variableCODER_E2E_LICENSE seems substituted through CI secret, but is it set correctly?toshikish/coder@set-min-max-for-workspace-parameters/.github/workflows/ci.yaml#L616

Yes, it is, but unfortunately, the secret is not available on PRs made through branches outside of coder/coder.

toshikish reacted with thumbs up emoji

@matifalimatifali requested a review fromsreyaOctober 29, 2024 12:45
@sreyasreya removed their request for reviewOctober 29, 2024 12:58
@github-actionsgithub-actionsbot added the staleThis issue is like stale bread. labelNov 6, 2024
@BrunoQuaresma
Copy link
Collaborator

I think rebasing this PR from main should fix the e2e tests.

@github-actionsgithub-actionsbot removed the staleThis issue is like stale bread. labelNov 7, 2024
@matifali
Copy link
Member

matifali commentedNov 7, 2024•
edited
Loading

@BrunoQuaresma unfortunately e2e tests can't work on forks as they don't have access to the repo secrets (the license).

Also we can't allow forks to have access to secrets in a safe way.

BrunoQuaresma reacted with thumbs up emoji

@matifali
Copy link
Member

@BrunoQuaresma COuld we run the e2e tests locally while we figure out a way to allow forks to run them?

@github-actionsgithub-actionsbot added the staleThis issue is like stale bread. labelNov 20, 2024
@matifalimatifali removed the staleThis issue is like stale bread. labelNov 22, 2024
@matifali
Copy link
Member

@BrunoQuaresma Could you review it again? Thanks

@matifalimatifali requested a review froma teamNovember 22, 2024 08:58
@BrunoQuaresma
Copy link
Collaborator

@matifali the code didn't change from my last review so I think we are ready to go. My only concern is not being able to run the e2e tests using the premium license.

@matifali
Copy link
Member

@matifali', the code didn't change from my last review,' so I think we are ready to go. My only concern is not being able to run the e2e tests using the premium license.

Can we run them locally and approve if they pass, and force merge? We are tracking e2e tests for forks in#15557

@BrunoQuaresma
Copy link
Collaborator

@matifali Sure, I just don't have permission to force a merge. We have integration tests for this so I think we are safe to merge it right now. If it breaks something in the main, I can jump on it right away.

matifali reacted with thumbs up emoji

@sreyasreya merged commite87b0bb intocoder:mainNov 22, 2024
24 of 28 checks passed
@toshikishtoshikish deleted the set-min-max-for-workspace-parameters branchNovember 26, 2024 01:25
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@BrunoQuaresmaBrunoQuaresmaAwaiting requested review from BrunoQuaresmaBrunoQuaresma was automatically assigned from coder/ts

Assignees

@toshikishtoshikish

Labels
communityPull Requests and issues created by the community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@toshikish@matifali@BrunoQuaresma@sreya
[8]ページ先頭
©2009-2025 Movatter.jp