Skip to content

Carlynn's Personal API#28

Open
Carlynn wants to merge 10 commits intoSF-WDI-LABS:masterfrom
Carlynn:master
Open

Carlynn's Personal API#28
Carlynn wants to merge 10 commits intoSF-WDI-LABS:masterfrom
Carlynn:master

Conversation

@Carlynn
Copy link

@Carlynn Carlynn commented Sep 5, 2017

Boxes on the page shift when you edit a field
If you edit a line or create a new item and it is longer than 2 lines the boxes shift

});


router.get("/", require("./controllers/index"));
Copy link

Choose a reason for hiding this comment

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

So much commented out code!! We let it slide the first couple projects but we're going to get nit-pickier about these things moving forward. You should make sure that you're not leaving in console.logs or commented out code.


});

function getVenueHtml(venue) {
Copy link

Choose a reason for hiding this comment

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

Make sure you're indenting properly - even when you're writing a template literal string like this! It makes it a lot easier to debug and insures the code is correct when it's appended to your index.html.

* DATABASE *
************/

var db = require('./models');
Copy link

Choose a reason for hiding this comment

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

You set up the variable "db" but you never use it...


// var db = require('./models');
var db = require('./models');
var venues = db.Venue;
Copy link

Choose a reason for hiding this comment

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

Even though you created the variable "venues", you still use db.Venue later in your controller functions so this seems unnecessary.

@spragala
Copy link

spragala commented Sep 5, 2017

You did a great job Carlynn! Your code looks great - just keep in mind it being cleaner going forward. Other than that the site looks awesome and functions perfectly.

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.

2 participants