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

Suggested updates to starter code #81

Closed
wants to merge 1 commit into from
Closed

Suggested updates to starter code #81

wants to merge 1 commit into from

Conversation

marcnjaramillo
Copy link

I have completed the major requirements for the Fyyur project, and I have made some updates to the starter code based on some issues I encountered while working on the project:

app.py

  • Reorganize the controllers and give each controller a distinct heading to make it easier to find each distinct route. I noticed that the /venues/<int:venue_id>/edit controllers where not listed with the other /venues/ controllers, and the /artists/create controller was listed after the /artists/<int:artist_id>/edit controller. I moved those around and made sure that the ordering was the same for both venues and artists. I also added headers for each separate controller so that finding a specific route to work on would be easier.

  • Adjust main section headers. This isn't really a necessary edit, but for me it helps keep the main sections and subsections clearly identified and feels cleaner.

/templates/forms/

  • Add missing form fields for new_venue, edit_venue, new_artist, edit_artist. All of these forms were missing several fields and were not rendering correctly, and since completing these forms was not listed as one of the requirements for completion I went ahead and edited these forms.

forms.py

  • Add missing attributes to VenueForm and ArtistForm. Several attributes were missing and causing the app to crash. I added the missing attributes and added the missing import for BooleanField. Again, since this file was not specifically marked as one needing revision to complete the project, I made the edits so other students won't encounter issues.

I hope the edits are useful and that I have appropriately explained the rationale behind them. If there is anything you would suggest changing, please let me know. Thank you!

@SudKul
Copy link
Contributor

SudKul commented Mar 16, 2021

Hi @marcnjaramillo
Though we are a little delayed in responding, we appreciate your contribution and a detailed description.
We have reviewed your proposed changes, and all of them look great. But, at the time of reviewing this PR, we have already updated the current repo with exactly same changes, as proposed in the #117

Thank you, for the good work you have done. 👍

@SudKul SudKul closed this Mar 16, 2021
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.

None yet

2 participants