sbp commented on code in PR #336:
URL:
https://github.com/apache/tooling-trusted-releases/pull/336#discussion_r2557642441
##########
atr/post/draft.py:
##########
@@ -37,37 +36,12 @@
import atr.web as web
-class VotePreviewForm(forms.Typed):
- body = forms.textarea("Body")
- # TODO: Validate the vote duration again?
Review Comment:
This comment was lost in the new class. Is the comment, or any of its three
lines, important? Should it have been retained, in part or in whole?
##########
atr/get/draft.py:
##########
@@ -44,7 +44,6 @@ async def tools(session: web.Committer, project_name: str,
version_name: str, fi
modified = int(await aiofiles.os.path.getmtime(full_path))
file_size = await aiofiles.os.path.getsize(full_path)
-
Review Comment:
Formatting changes should be reserved for a separate PR. We prefer minimal
PRs because that makes them easier to review.
##########
atr/shared/__init__.py:
##########
@@ -21,10 +21,12 @@
import atr.db as db
import atr.db.interaction as interaction
-import atr.forms as forms
+import atr.form as atr_form
Review Comment:
We don't change interface names when we import them.
##########
atr/shared/draft.py:
##########
@@ -15,21 +15,29 @@
# specific language governing permissions and limitations
# under the License.
-import atr.forms as forms
+import pydantic
+import atr.form as form
-class DeleteFileForm(forms.Typed):
- """Form for deleting a file."""
- file_path = forms.string("File path")
- submit = forms.submit("Delete file")
+class DeleteFileForm(form.Form):
+ file_path: str = form.label(description="File path")
-class DeleteForm(forms.Typed):
- """Form for deleting a candidate draft."""
+class DeleteForm(form.Form):
+ release_name: str = form.label(description="Release name",
widget=form.Widget.HIDDEN)
Review Comment:
The `description=` keyword should be removed from all of these `label` calls.
##########
atr/shared/draft.py:
##########
@@ -15,21 +15,29 @@
# specific language governing permissions and limitations
# under the License.
-import atr.forms as forms
+import pydantic
+import atr.form as form
-class DeleteFileForm(forms.Typed):
- """Form for deleting a file."""
- file_path = forms.string("File path")
- submit = forms.submit("Delete file")
+class DeleteFileForm(form.Form):
+ file_path: str = form.label(description="File path")
Review Comment:
We don't use keywords for the arguments to `form.label`. As mentioned on my
recent #309 review, the general pattern here is that we don't use keywords for
arguments before `*`, but that might not apply globally. We should probably
check.
##########
atr/shared/draft.py:
##########
@@ -15,21 +15,29 @@
# specific language governing permissions and limitations
# under the License.
-import atr.forms as forms
+import pydantic
+import atr.form as form
-class DeleteFileForm(forms.Typed):
- """Form for deleting a file."""
- file_path = forms.string("File path")
- submit = forms.submit("Delete file")
+class DeleteFileForm(form.Form):
+ file_path: str = form.label(description="File path")
-class DeleteForm(forms.Typed):
- """Form for deleting a candidate draft."""
+class DeleteForm(form.Form):
+ release_name: str = form.label(description="Release name",
widget=form.Widget.HIDDEN)
+ project_name: str = form.label(description="Project name",
widget=form.Widget.HIDDEN)
+ version_name: str = form.label(description="Version name",
widget=form.Widget.HIDDEN)
+ confirm_delete: str = form.label(description="Type DELETE to confirm")
- release_name = forms.hidden()
- project_name = forms.hidden()
- version_name = forms.hidden()
- confirm_delete = forms.string("Confirmation",
validators=forms.constant("DELETE"))
- submit = forms.submit("Delete candidate draft")
+ @pydantic.field_validator("confirm_delete")
+ @classmethod
+ def validate_confirm_delete(cls, value: str) -> str:
+ if value != "DELETE":
+ raise ValueError("You must enter 'DELETE' in this field")
+ return value
+
+
+class VotePreviewForm(form.Form):
+ body: str = form.label(description="Body", widget=form.Widget.TEXTAREA)
+ vote_duration: form.Int = form.label(description="Vote duration")
Review Comment:
Should not use `description=`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
