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

Import: remove extra/advanced step from project import wizard #10603

Merged
merged 2 commits into from Aug 18, 2023

Conversation

humitos
Copy link
Member

@humitos humitos commented Aug 6, 2023

This commit removes the "Extra/Advanced" step and moves the "language" attribute to the "Basics" step -- the first one, since that field is important.

Closes #10392

This commit removes the "Extra/Advanced" step and moves the "language" attribute
to the "Basics" step -- the first one, since that field is important.

Closes #10392
@humitos humitos requested a review from a team as a code owner August 6, 2023 12:07
@humitos humitos requested a review from stsewd August 6, 2023 12:07
@stsewd stsewd requested a review from agjohnson August 16, 2023 17:48
Copy link
Member

@stsewd stsewd left a comment

Choose a reason for hiding this comment

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

LGTM, I could also see having description and homepage listed, maybe grouping them using crispy forms? Probably something for the new templates. I'm including Anthony in the review, he may have some thoughts here.

@agjohnson
Copy link
Contributor

On the new dashboard side, all forms are just displayed via {{ form|crispy }}, so should be fairly easy to alter these forms on the new dashboard if we're doing the same thing here.

We also did talk about restructuring more of the projects settings in the near future, so could keep some of the changes for later too. I think we'd like more project admin tabs/pages, so if any of these fields feel out of place anywhere, maybe those are ones to consider moving out.

maybe grouping them using crispy forms

I need to play around with this more, but currently the crispy forms don't do fieldsets much at all. It should be okay normally though, I expect there is just some missing template overrides for the fieldsets in the crispy-sui form templates

I could also see having description and homepage listed

On the project settings dashboard these are probably good fields to have. But on the import page, perhaps we want to be relying on GitHub and friends to give us as much of these fields as we can? The could mean not presenting these fields to users on remoterepo connected project creation.

No strong feeling here either way though.

@humitos
Copy link
Member Author

humitos commented Aug 17, 2023

I could also see having description and homepage listed

on the import page, perhaps we want to be relying on GitHub and friends to give us as much of these fields as we can?

Yes. This was my feeling here as well and that's why I decided to not include them for now. Besides, we have talked about deprecating the Project.description field in #3689.

I'm fine moving forward with this PR as-is and update it later if needed.

Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

This all looks reasonable to me. I haven't tried these changes to the UI yet, but don't expect much difference in either dashboard.

@humitos humitos merged commit 2f4334d into main Aug 18, 2023
7 checks passed
@humitos humitos deleted the humitos/remove-extra-wizard-import branch August 18, 2023 10:05
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.

Project import: "Edit Advanced Options"
3 participants