-
Notifications
You must be signed in to change notification settings - Fork 54
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
Review #37
base: master
Are you sure you want to change the base?
Review #37
Conversation
@@ -49,12 +49,12 @@ app.get('/api', function apiIndex(req, res) { | |||
res.json({ | |||
woopsIForgotToDocumentAllMyEndpoints: true, // CHANGE ME ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you have updated all the below lines, you can change this flag to false. Or probably delete the whole line altogether so it does not show up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You just need to code up the app.js now to fill in the missing link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like you didn't get to working code over the weekend, which is concerning to me.
|
||
var db = require('../models'); | ||
|
||
// // GET /api/skatespots |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this code is mostly-working, but all commented out--I'm confused by the state this code was left in.
|
||
// var new_campsite = {description: "Sharp rocks. Middle of nowhere."} | ||
var new_skatepark = {description: "Sharp rocks. Middle of nowhere."} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like you didn't actually edit your seed file to be relevant to the structure of your skatespots.
<div class="row"> <!-- a row just sets up the containters --> | ||
<!-- offset is spacing, offest by one leaves the 1st container empty | ||
and starts the div one over from the left, since there are 10 | ||
it centers it, leaving one column empty on both sides --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These comments are awesome!
<!-- offset is spacing, offest by one leaves the 1st container empty | ||
and starts the div one over from the left, since there are 10 | ||
it centers it, leaving one column empty on both sides --> | ||
<div class="col-lg-10 col-lg-offset-1"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm concerned that you're using col-lg in some places, and col-md in others--you generally want to use the smallest possible size for everything, so make all of them col-md or col-sm.
To be honest, I was confused. Friday it was being said that it didn't have
to be finished. When Matt said he didn't finish his and that we weren't
expected too it led me to believe that it didn't have to. So I made the
mistake of not finishing. ------- I like using LG because it gave better
spacing visually than the other ones. ---------- Also, I was confused on
what kind of framework our programs were supposed to have, because it
changed from project to project. On this project we are making, it became
clear that I really had no idea on a concrete way to set up my work. So
that is what I have had to focus on in this project. In working with Arun,
I believe I finally have a clear outline on how our projects are supposed
to look. Am I right, or will they continue to vary from project to project
when we are using node and mongo? I have been lost the past week and a half
before this week, lost on how to setup my apps and where to go with them.
…On Thu, Sep 7, 2017 at 2:24 PM, Michelle Ferreirae ***@***.*** > wrote:
***@***.**** commented on this pull request.
It seems like you didn't get to working code over the weekend, which is
concerning to me.
------------------------------
In controllers/skatespotController.js
<#37 (comment)>
:
> @@ -0,0 +1,55 @@
+/************
+ * DATABASE *
+ ************/
+
+var db = require('../models');
+
+// // GET /api/skatespots
It seems like this code is mostly-working, but all commented out--I'm
confused by the state this code was left in.
------------------------------
In seed.js
<#37 (comment)>
:
>
-// var new_campsite = {description: "Sharp rocks. Middle of nowhere."}
+var new_skatepark = {description: "Sharp rocks. Middle of nowhere."}
It seems like you didn't actually edit your seed file to be relevant to
the structure of your skatespots.
------------------------------
In views/index.html
<#37 (comment)>
:
> </div>
</div>
+
+ <!-- Begin form Section -->
+ <section id="skatepark-form" class="container">
+ <!-- a row can be any size in height, grows bootstrap -->
+ <div class="row"> <!-- a row just sets up the containters -->
+ <!-- offset is spacing, offest by one leaves the 1st container empty
+ and starts the div one over from the left, since there are 10
+ it centers it, leaving one column empty on both sides -->
These comments are awesome!
------------------------------
In views/index.html
<#37 (comment)>
:
> </div>
</div>
+
+ <!-- Begin form Section -->
+ <section id="skatepark-form" class="container">
+ <!-- a row can be any size in height, grows bootstrap -->
+ <div class="row"> <!-- a row just sets up the containters -->
+ <!-- offset is spacing, offest by one leaves the 1st container empty
+ and starts the div one over from the left, since there are 10
+ it centers it, leaving one column empty on both sides -->
+ <div class="col-lg-10 col-lg-offset-1">
I'm concerned that you're using col-lg in some places, and col-md in
others--you generally want to use the smallest possible size for
everything, so make all of them col-md or col-sm.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#37 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AI2GqkfU4S784mGrUauQ2_8ltDPBiOw2ks5sgF8rgaJpZM4PNNs->
.
|
Using col-lg will only apply once the screen is at least 992px. It should have no visual difference, once the screen is that size, between if you used col-lg or col-md. (If that's the look you were going for, my apologies!) Also, it's true that we have used two different basic file structures in setting up our apps. I do think you're in a reasonable place, in terms of those setup steps, but I'm happy to talk more about this in person too. We will definitely continue to use models, controllers, view code, frontend JS, CSS files, and all the rest going forward; all the other differences between projects are minor, and are there to show you that there isn't just one way of making a project work. |
No description provided.