Skip to content

Conversation

@jarrodu
Copy link
Member

@jarrodu jarrodu commented Nov 22, 2016

See #546

@jarrodu jarrodu mentioned this pull request Nov 22, 2016
@jarrodu
Copy link
Member Author

jarrodu commented Nov 23, 2016

I am going to go ahead to merge this in. Let me know if I was a bit to hasty. :-)

@jarrodu jarrodu merged commit f2e024b into scala:master Nov 23, 2016
@heathermiller
Copy link
Member

Yeah, it was a bit too hasty. I didn't have time to go through your layout changes. It would really help to have screenshots or explanations of what you did and why.

@jarrodu
Copy link
Member Author

jarrodu commented Nov 23, 2016

Sure no problem. How do I revert the commit?

@heathermiller
Copy link
Member

No need to revert. Could you just explain what the rationale was for the layout changes? Worst case if we need to tweak anything, we can do in a follow up (probably don't have to though :))

@jarrodu
Copy link
Member Author

jarrodu commented Nov 23, 2016

Okay sounds good. I will prepare something in a few minutes.

@jarrodu
Copy link
Member Author

jarrodu commented Nov 23, 2016

The motivation of these changes is to give a future contributor a cleaner and simpler source code directory to work with. There are rumors that a person may be hired to redo the websites. So making it as easy as possible for this person to get up and running seems like a great idea.

In the first pass I went through and removed any files that seemed old and unused. There were quite a few files that had not been touched in years so they were removed. I also removed a lot of commented out code with the idea that anything was actually needed could be retrieved from the file histories.

Some larger changes were made to the templates to reduce duplicated code and to make it easier to reason about. For instance a base template was added. The default template was blank and did nothing so it was removed and all references to it were also removed. Some html pages where nearly bank and all the content of the page was in a template file. This was changed. The index page for the downloads directory is an example of this. The main index for the site is still like this and I will likely add an issue at some point about it.

There was also a small design change that again was mostly motivated by the desire to simplify the templates. I added a table of contents to a few pages that did not have them previously. I think this is also a good design choice since it keeps each page on the site consistent with the other pages.

I also removed quite a few yaml variables that were not being used in the pages.

I probability should have typed this up when I first introduced the PR instead of writing it after the PR got merged into master. Live and learn.

@heathermiller
Copy link
Member

This all sounds good to me. Thanks for the rationale :)

@heathermiller
Copy link
Member

heathermiller commented Nov 23, 2016

I looked through the changes yesterday, most stuff seemed like a good idea. You worried me a bit on the tocs, but otherwise most changes looked sensible to me.

The only thing I'm concerned about is the removal of _tools/redirections.text. I thought that something (not in this repository) was using that serverside. Maybe @fsalvi knows?

@fsalvi
Copy link
Contributor

fsalvi commented Nov 23, 2016

No, it's ok for the redirections, the file is not used on the server...

jarrodu added a commit that referenced this pull request Nov 26, 2016
rename "base" template to "default" template

Expand on changes made in #550 to help simplify the templates.
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.

3 participants