nellarro / build-it-together

Midterm
0 stars 4 forks source link

Fixed stretched images with header modification #23

Closed thetechtakeover closed 4 years ago

thetechtakeover commented 4 years ago

Found and replaced original images with new images that are the same size. The a few of the original images were small which caused them to stretch when placed 100% in container. I also added a hero banner and title to header of webpage and edited media query for banner

nselikoff commented 4 years ago

@thetechtakeover awesome, thanks for tackling this! Before we review I'd like to request that you rebase against master. This PR is a good opportunity to do so for a couple of reasons:

So to do the rebase:

  1. From your master branch, run git rebase master (command line), or use your IDE's or Github Desktop's equivalent command
  2. There may be conflicts, which you should be notified of during the rebase process. If there are conflicts, you'll need to fix them, and then finish the rebase
  3. Force push to your master branch: git push -f

At this point the PR should automatically update and the merge commits should disappear.

You can read more about git rebase here: https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase

Let me know if you have any questions!

thetechtakeover commented 4 years ago

@nselikoff Good Evening,

I think I followed the instructions and even watched a video about rebase. After doing a pull "rebase" Its telling me that my branch is "up to date with "origin/master". I did a push to my master as well after the rebase but I still see the commits

nselikoff commented 4 years ago

Whoops! I made a mistake on the commands.

  1. Make sure you've fetched the latest from upstream: git fetch upstream
  2. Rebase against upstream master: git rebase upstream/master
  3. Fix conflicts, mark as resolved, and run git rebase --continue
  4. Force push: git push -f
nselikoff commented 4 years ago

If you run the rebase in the command line, you should see something like the following (I'm not sure where/how this shows up if you're using a GUI tool but it probably is similar):

Auto-merging main.css
CONFLICT (content): Merge conflict in main.css
Auto-merging index.html
CONFLICT (content): Merge conflict in index.html
error: could not apply b88c2cd... added structure to html file / css file
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply b88c2cd... added structure to html file / css file

This message points out where the conflicts are and what you need to do to mark them as resolved. And it also gives you an escape hatch to cancel the rebase.

thetechtakeover commented 4 years ago

@nselikoff whew!!! Listen, I went through the steps even though I wasn't 100% comfortable not knowing what I was really doing! lol But I ran the new commands you listed. I usually go between VSCode and github desktop honestly when i get stuck but I usually make it happen.

I ran the commands through the VSCode terminal and as usual any changes my github desktop picked it up. So the github desktop started to run the rebase to upstream. I went through the process and like you mentioned, when conflicts came up, github desktop pointed me back to VSCode to resolve/fix. After I fixed about 6-8 conflicts it seemed to complete and stated that there are no more conflicts. I then used the terminal in VSCode to git status (everything returned back as good and up to date) and then I " git push -f ". Everything seemed to go through but now im back on github and still see the commits. :(

nselikoff commented 4 years ago

Nice job, you did it right! You'll notice there are still multiple commits, but the intermediate merge commits that used to be there are gone. Multiple commits are fine, unless someone asks you to "squash" which is another thing you can learn about and can do in conjunction with rebasing: https://medium.com/@slamflipstrom/a-beginners-guide-to-squashing-commits-with-git-rebase-8185cf6e62ec

thetechtakeover commented 4 years ago

@nselikoff ok no problem, I was under the impression that someone else was working on the responsive part so I didn't want to make many changes. Thats no issue, I can back in and work on the images being responsive soon.

nselikoff commented 4 years ago

@thetechtakeover that's understandable - my recommendation would be to let @nojocode work on the responsive part, and you work on the images not being stretched out of proportion (these can be tackled separately even though the reference I provided will talk about both). Another good search term if you're stuck on this is "aspect ratio".

thetechtakeover commented 4 years ago

Fix stretching image issue @nselikoff

nselikoff commented 4 years ago

Nice job @thetechtakeover