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

blazor-crud scaffolder minor fixes #2938

Merged
merged 4 commits into from
Aug 24, 2024
Merged

Conversation

deepchoudhery
Copy link
Member

Addressing issues from #2743

  • extra semicolon in builder.Services.AddQuickGridEntityFrameworkAdapter(); : fixed earlier
  • fixed directive orders and change properties + methods to private issue: fixed
  • nit issues : 1, 2, 3, 4
  • Index.razor issue regarding QuickGrid.Items assignment : fixed earlier creating and disposing DbContext correctly in Index.razor now #2896
  • json issue : not a scaffolding issue since this change is not caused during scaffolding, these are files created by the new project templates.
  • primary constructor issue : not fixing in this PR since I have a lot of places this needs to be fixed for. It is recommended but I need to fix it scaffolding-wide in a different PR

Copy link

@guardrex guardrex left a comment

Choose a reason for hiding this comment

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

Thanks @deepchoudhery ... I just see a handful of NIT suggestions.

WRT my Mackinnon ❓, I think you did the right thing making the properties private. I'll probably need to adjust our examples in the main doc set based on what he says.

WRT the primary ctor, no problem. I dealt with that one in the tutorial by not showing the code for it. It's free to change at any time without requiring a tutorial update later.

Copy link
Member

@danroth27 danroth27 left a comment

Choose a reason for hiding this comment

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

I agree with @guardrex 's suggestions. Otherwise, these changes looks good to me.

@deepchoudhery deepchoudhery merged commit fddcc88 into main Aug 24, 2024
8 checks passed
@deepchoudhery deepchoudhery deleted the dev/decho/blazor-crud-fixes branch August 24, 2024 02:08
deepchoudhery added a commit that referenced this pull request Aug 27, 2024
* blazor-crud scaffolder minor fixes

* PR comment fixes 1

* changing heading sequence

* minor fix
deepchoudhery added a commit that referenced this pull request Aug 27, 2024
* blazor-crud scaffolder minor fixes

* PR comment fixes 1

* changing heading sequence

* minor fix
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.

5 participants