freeCodeCampGuam / freecodecampguam.github.io

The website for Free Code Camp Guam
https://freecodecampguam.github.io/freecodecampguam.github.io/
0 stars 4 forks source link

Multiple pages #24

Closed ghost closed 7 years ago

ghost commented 7 years ago

I've created js, css, and html folders that will store respective file extensions in each folder, and I've linked external js, css, and html files.

I've created a header.html file which is our navbar, and it'll be loaded onto page with jQuery.

For now, we'll be using the front-end to create multiple pages. Eventually, we'll be able to take care of routing once we pick a server-side language.

Comment below if you have any questions.

ghost commented 7 years ago

This will be merged tonight unless someone rebukes.

Chovin commented 7 years ago

Technically unfinished since we can't see if the header can be added in through the addition of jQuery. It is an improvement though. once our page is deployed to the public, let's not let PRs like this merge.

ghost commented 7 years ago

What do you mean we can't see if the header can be added in through the addition of jQuery?

Chovin commented 7 years ago

this PR makes a promise that something it removed from the user's vision will be able to be put back in later. it's an unfinished improvement. ie. it's in development. it should be in a development branch. technically it's "breaking" something.

It's an improvement to the current structure though and our site isn't deployed yet so it's fine for now but, imo, PRs like this shouldn't be accepted in the future if we're following Github Flow

ghost commented 7 years ago

The PR doesn't remove anything from the user's vision. I just moved the header into a separate file that'll be loaded onto all pages via jQuery. I don't really understand what you mean, sorry.

ghost commented 7 years ago

Closing PR to edit features before making another PR.

Chovin commented 7 years ago

my mistake, just didn't have webserver up when testing and didn't see the js you added :3

jasperdanan commented 7 years ago

d2b675c Added in this branch: custom.css for any bootstrap override

Chovin commented 7 years ago

@gabrielcerteza we can't check the PR if you keep committing to it. please comment on this -> #27

ghost commented 7 years ago

I only moved Jasper's changes into a consolidated css file.

Chovin commented 7 years ago

you said to keep the branch until the feature is complete in #27. Please request a code review once it is complete so we know when to review and give our 👍

ghost commented 7 years ago

If two people pushed two different features to the branch, then shouldn't we just merge it already? We're only missing the projects page because we don't have a design for it yet. I'll just link it to some random page and prompt for a code review.

On Wednesday, 26 October 2016, Chovin notifications@github.com wrote:

you said to keep the branch until the feature is complete. Please request a code review one it is complete so we know when to review and give our 👍

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/FreeCodeCampGuam/freecodecampguam.github.io/pull/24#issuecomment-256307210, or mute the thread https://github.com/notifications/unsubscribe-auth/AQOxOdCHtyRG0nfbCOH5Og9U6R9XP8Qfks5q3yltgaJpZM4KfhyG .

Chovin commented 7 years ago

No. PRs should only be merged if 2 or more people have reviewed all the changes made. As soon as one person pushes to this branch the reviewing process has to start over. Keep doing that over and over and you're just wasting people's time.

Making a PR for review and convo is fine but you should call off the time limit if you're going to continue working on the PR

jasperdanan commented 7 years ago

Any update on the pull request?

Chovin commented 7 years ago

I thought this was a WIP pr? @gabe, what are your intentions with this PR?

jasperdanan commented 7 years ago

Confirming PR