Random images

[edwargix]

Resolves #26

[edwargix]

I think I correctly changed what you all wanted. Let me know if this isn't the case.

[jackrosenthal]

Also, we can get rid of banner.jpg now.

[edwargix]

Once we perfect this, do we want a rebase of the branch to fixup the commits and have the history a little cleaner? (i.e. not have changes that get reverted 2 commits later)

[sumnerevans]

@edwargix: Yes. Rebase for sure. Clean up as much as necessary, but don't go overboard on it. (It's fine to have a couple commits see #33 for example.)

[jackrosenthal]

Please be careful about this rebasing Sumner is reccomending. Generally, it's not a good thing if you need to force push to the repo (it's like trying to rewrite history!) Especially, never force push to master without contacting me first.

Sumner has been known to over-rebase in the past. Just because he does it a lot does not mean it's a good thing...

[edwargix]

Would it be best to rebase onto a new branch, like random-images-rebase?

[sumnerevans]

@jackrosenthal, the master branch is already protected and no force pushes are allowed. Merges are fine. Honestly, I actually prefer it, I thought that you liked it better (maybe I'm confusing you and Sam)

[jackrosenthal]

If you cannot justify why you should rebase, then don't. It's pretty simple.

What will you get out of rebasing? It makes sense for things like cleaning up binaries accidentally comitted, but you definately overuse it. Essentially, just justify why you want to rewrite history, then it should be OK. But not allowed without justification.

[edwargix]

Ok the rebase is done and can be merged unless there's something else I'm missing. I have a local copy of the branch prior to the rebase, so if anyone needs that just let me know.

[jackrosenthal]

PLEASE DO NOT REBASE WITHOUT JUSTIFYING FIRST!!!

[edwargix]

I rebased because there was a lot of back-and-forth between commits. The end code is identical and works the same.

[jackrosenthal]

Yes, but from what I could tell, the commits told a history. You only need to rebase if binaries are committed, you're smashing up wip commits, or other things like that. There's no reason to destroy incremental steps of history in this thing otherwise.

[sumnerevans]

I'll take the blame on this one. But Jack, your hard-line on no rebasing is a detriment to the cleanliness of the commit tree.

[edwargix]

If we prefer the history with wip commits, I can reset the head of this branch back to that.

[jackrosenthal]

The commit tree tells history. If that is destroyed, then the commit tree is uncleanly, the history has been removed. There are no WIP commits from what I can tell, so please do not rebase.

[edwargix]

I already did rebase, so should I reset this branch's head back to what it was before?

[jackrosenthal]

Yes

[edwargix]

Done

[sumnerevans]

@jackrosenthal, it's perfectly fine for you to take a hard line on these matters of project management. However, without a CONTRIBUTING.md specifying your direction, I cannot guarantee that I or anyone else will do exactly what you want.

Again, I'm taking the blame on this one, but you continue to take sides on issues and expect us to be able to read your mind which is not a sustainable project management strategy.

(Sorry for having to be caught up in this @edwargix.)

[jackrosenthal]

Sorry, I had no idea that one would do anything without justification, let alone rebase without justification, when working on the repository. Doing things without justification is a pretty good definition of bigotry.

[sumnerevans]

@jackrosenthal, it is equally, if not more, bigoted to consider a justification you don't agree with to be invalid and nonexistent. @edwargix and I made it clear that the reason to rebase was to remove back and forth commits where things were changed and then immediately reverted back.

[jackrosenthal]

Back and forth commits show history. They weren't WIP commits that were quickly changed. They weren't binary commits accidentally commited. It was good these were separate commits!

[samsartor]

@jackrosenthal I have been staying out of this so far, but I have to agree with @sumnerevans on this one. Different repos use different strategies. I have worked on projects where rebasing and squashing was required before merging. If you have a preference you absolutely must put it in CONTRIBUTING.md.

[jackrosenthal]

Never assume.

Repositories which require that specify that. Repositories which don't require that (and don't want it) should not be required to specify that simply because other projects implement that.

In other words, the lack of a specification means something too!

[sumnerevans]

@jackrosenthal,

Re "back and forth": case and point. The word "good" always implies judgment. That is your opinion. This is your project, so you have every right to impose your opinion on this project. However, assuming that we are all on the same page is extremely narrow-minded.

Re "never assume": you should never assume that we can read your mind. If rebasing is something you are so passionate about, then why would you not want to specify it it a CONTRIBUTING.md? If you want to take a hard line on something, specify it in writing. PEP8 is an example of a document which clearly defines where hard lines exist, even if they are obvious (i.e. no tabs, use 4 spaces for indentation).

[samsartor]

@sumnerevans @jackrosenthal PR comments are not the right place to have this argument. This is what chats are for.

[sumnerevans]

On the actual PR, I'm not sure the best way to fix this, but there are some problems on small screens with this style. (Note that the picture is not inside the Jumbotron. Not sure if we want that or not.)

Also, the text seems a bit big, and there is possibly too small of a margin with the picture.

I think we should go with what Jack suggested in this thread: https://github.com/ColoradoSchoolOfMines/acm-website/pull/32#discussion_r154493443, and revert back to what we had before (just whitespace there when no image is found).

What's your say on this @jackrosenthal?

[jackrosenthal]

What's your say on this @jackrosenthal?

Pay careful attention to "when there is no image".

[jackrosenthal]

Honestly think we should just revert back to what we had before (a very good reason to keep commit history on things like this).

If there was to be a jumbotron, it would have been where the missing picture is, not where the text on the right is.

[jackrosenthal]

Looks good to me

[jackrosenthal]

Ready to merge?