Skip to content
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

Revamped Website 2019 #9

Closed
wants to merge 308 commits into from
Closed

Revamped Website 2019 #9

wants to merge 308 commits into from

Conversation

gmanlan
Copy link
Contributor

@gmanlan gmanlan commented Nov 16, 2019

No description provided.

@rcurtin
Copy link
Member

rcurtin commented Nov 21, 2019

Rebuilt and deployed. 👍

@gmanlan
Copy link
Contributor Author

gmanlan commented Nov 22, 2019

Rebuilt and deployed. 👍

@rcurtin : did you re-build the Doxygen doc as well? I see the logo missing in the doc header, as well as an extra (removed already) github link. Maybe because the doc is using a previous header?

@rcurtin
Copy link
Member

rcurtin commented Nov 23, 2019

@gmanlan good point, I missed that part. I'm rebuilding the doxygen versions now, but at least the binding documentation has the headers correct now. Sorry about that.

Copy link
Member

@rcurtin rcurtin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @gmanlan,

This is really awesome. I think that the previous website wasn't great, and it's pretty clear I am not a website designer. So I really appreciate the effort you've put in here. I do have a handful of comments now that I look through the site closely; let me know what you think and if I can be helpful with them. A lot of website design is opinion, so hopefully we can come to something in the end that we all reasonably agree upon. :)

My biggest subjective comment is that I think we should try and think of a way to relate the imagery/design on the homepage to something more "machine learning" or "math"-y. I guess probably the reason for that comment is mostly the hero image, but maybe there are some other things we could do also, not sure. I'll think about some other ideas, but I don't know what I'll be able to come up with.

There are a couple other things that need to happen too before merge, but these are on my end:

  • I need to figure out what is up with the git conflicts so I can actually push the existing mlpack.org back to this repo.
  • We need to figure out how to remove the git LFS datasets from here.
  • I need to update the doxygen build scripts to not negate all the images.
  • I need to adapt some of the URLs/versions so that they are automatically generated by the website build script.

Many of these things we can address by opening issues and coming back to them in the future, so that we can get the new website deployed reasonably soon. So it's not an issue from my end if we decide for many of the comments that they can be handled at another time.

Anyway, I'm pretty excited about this. 🚀

_includes/header.html Outdated Show resolved Hide resolved
_layouts/default-nav.html Outdated Show resolved Hide resolved
_sass/docs.scss Outdated Show resolved Hide resolved
_sass/layout.scss Outdated Show resolved Hide resolved
_sass/typography.scss Outdated Show resolved Hide resolved
}

.overload {
font-family: "courier new",courier,monospace;
font-family: "courier new",courier,monospace;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, should we use Roboto here? Looks like I might have overlooked that in the past.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, but we should see how it looks like once you rebuild doxygen.

_src/doxygen/doxygen.css Outdated Show resolved Hide resolved
docs.md Show resolved Hide resolved
docs.md Outdated Show resolved Hide resolved
getstarted.md Outdated

Here is a summary of the currently available distribution options you can use depending on your needs:

<h2>Python</h2>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this file also I think we can replace all the <hX> with Markdown headings, and the links with Markdown links.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Could you please review all the links and update them accordingly? When I started this project we were referring to mlpack 3.1 and new versions have been released. Also, I believe we should be using a static link instead of versioned (i.e. stable).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't do my homework, which was to figure out how to push the current state of mlpack.org back to the repository. I'll do that now, but I know it will cause merge conflicts on this branch. I can work them out too and push the changes / provide a patch, or just work it out during the actual merge of the PR; up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will be faster if you solve the conflicts and/or manually update the files to use the correct links. Then moving forward we can make sure it's a seamless process.

@zoq
Copy link
Member

zoq commented Nov 24, 2019

My biggest subjective comment is that I think we should try and think of a way to relate the imagery/design on the homepage to something more "machine learning" or "math"-y. I guess probably the reason for that comment is mostly the hero image, but maybe there are some other things we could do also, not sure. I'll think about some other ideas, but I don't know what I'll be able to come up with.

I have a couple of ideas that could work here, I'll sketch something up and post an update later today.

@zoq
Copy link
Member

zoq commented Nov 24, 2019

I tested out different ideas and combined some of them that you can see in the example images below. Also, I haven't touched anything on the subpages, I think they are great so I would probably only change the color-scheme.

In the last meeting, we had a discussion, about how cool notebooks are, so I tried to pick that idea up for the frontpage. So the editor you see isn't a gif or video but a simple editor (ace) that acts as a frontend to run code on a remote machine (in a docker container or something). The output is shown in the white box below. Note the output doesn't match with the example code. So a user could use that as a simple and quick way to run mlpack without installing anything.

I thought the focus wasn't on the actual code example with the reddish polygon background, so I tried to find a more nutural color that could work, unfortunately, I couldn't find a reddish color so I went with blue.

I've updated some of the sections below as well, like the citation section, which now shows more than one paper and added links to the BibTeX file, google scholar and the pdf file; not sure we should list more than one paper.

Haven't really changed the cards; slightly decreased the corner-radius and adapted the color scheme.

There is also a new benchmarks section, I'm not sure if I like it or not; the current minimal approach might be better.

As Ryan pointed out we should add a NumFOCUS section as well, so I did that and just copied the text from the Github page.

Interested to hear what you think about some of the ideas.

alt text

alt text

alt text

@gmanlan
Copy link
Contributor Author

gmanlan commented Nov 25, 2019

Hey @zoq: thanks for sharing. I like the ideas of adding NumFocus and Benchmark data, that surely adds value to the table and I couldn't find that information before, so I'm glad you bring it.
On the other side, it seems you have redesigned the website, and therefore we would need to "start again". I spent a couple of months working on this version (that we discussed over here: mlpack/mlpack#1892) and unfortunately I won't have more time to re-do it again. If folks like the blue design better, would you have time and bandwidth to re-do it?

@rcurtin: I will hold the code changes from your review until we decide on this.

Cheers

@rcurtin
Copy link
Member

rcurtin commented Nov 25, 2019

@gmanlan I think maybe @zoq's changes can be done in such a way that they're pretty minor. Really it seems like just some CSS tweaks plus some tweaks to the header HTML on the front page. (For instance, can the red/blue change be done with just some simple regex or a quick SCSS change? I'm not sure.) But if @gmanlan is short on time, we should maybe try to make the changes minimal here and then address things like adding the notebook interface / benchmarks / style changes in separate PRs?

From my opinion I see awesome ideas in both designs and I think the ideas are pretty complementary:

  • I really like the cards and flow of @gmanlan's design.
  • I like the red/black coloring of @gmanlan's design since that sticks with the logo colors. Maybe the blue can be replaced with a subtle red tone or maybe even a gray tone in @zoq's mockup? I don't have a strong opinion.
  • The SCSS improvements from @gmanlan are really nice, apparently I was not very good at using SCSS to its full power.
  • I like the notebook idea from @zoq, although we should be careful that we run significant amounts of code for every visitor to mlpack.org... we'll light some machines on fire like that. Logs indicate that about 4k unique IPs visited mlpack.org yesterday.
  • I like the idea of adding benchmarks from @zoq's design, or maybe a card to benchmarks where we can have a more complete page or something?

Maybe we should proceed like this---once we can find some consensus on which parts of each design we should take and how we should fit them together, @gmanlan if you want to point out the comments I left that you'd like to take care of / leave for later, we can resolve the ones for later and open Github issues for them. Then we can merge this, and @zoq could open issues/PRs for additional changes for other parts of the design. In the past the mlpack.org repo has been pretty ignored (I've just committed various changes as needed), but it wouldn't be a bad idea to actually start tracking the things that we want to change.

Anyway, let me know what each of you think. I'm excited that we are all seeing great ways to improve the website, and I think no matter what we come out with something that's better in basically every way. (If you want to see how far we've come, here's a blast from the past: http://web.archive.org/web/20120607071412/http://www.mlpack.org/. Another testament to the fact that I am not a designer. :))

@gmanlan
Copy link
Contributor Author

gmanlan commented Dec 3, 2019

@rcurtin and @zoq: I agree with the design change proposals and additions.
As you said, I believe we should tackle major changes with future issues/PRs and with more hands on deck. As far as I know, we all agreed on the initial design of the website and all the interested parties provided feedback when discussed months ago: mlpack/mlpack#1892 (comment)

If we introduce radical changes at this point, we will have been wasting months of (interrupted) work. 😊 Anyway, if somebody wants to take over, feel free.

Said that, I think I'm done with the code review changes. I resolved most of the comments but I'm leaving a couple of them opened so you can check/review or act on an AR.

I may have 1 extra day for final polishing, but I'm afraid I won't be available anymore for a while.

Looking forward to hearing from you.

@rcurtin
Copy link
Member

rcurtin commented Dec 4, 2019

Blah, well, crap. I force pushed the original repository (because I needed to ensure that it wasn't using LFS storage and didn't have large files) and then Github closed this PR and I can't reopen it. That's not what I wanted. Certainly during merge, I would have to probably cherry-pick everything. So I can do that now. @zoq @gmanlan let me know when each of you are done with the comments and I'll go ahead and cherry-pick to do the merge and deploy the new website. We can still keep commenting on the closed PR (it seems we won't have any other choice... sorry about that).

@zoq
Copy link
Member

zoq commented Dec 4, 2019

I'm happy with the current design, so whenever you are ready let's deploy the new website. Also, I like the idea to open separate PR's for further changes.

I guess, we can setup some sort of publish pipeline that makes it easier to deploy new changes?

@rcurtin
Copy link
Member

rcurtin commented Dec 4, 2019

Yeah, definitely, I'll set up a job on ci.mlpack.org to operate just like the ensmallen website.

@gmanlan
Copy link
Contributor Author

gmanlan commented Dec 5, 2019

@rcurtin, @zoq: I'm done with the changes from the comments. Just pushed one last commit so we should be ready to go (hopefully you can pull that even though the PR is closed). Please go ahead with the deployment and feel free to comment back here if you encounter issues. Thanks!

@rcurtin
Copy link
Member

rcurtin commented Dec 5, 2019

@gmanlan awesome, thanks. I'll cherry-pick the commits, do a test deployment, then deploy to mlpack.org, and then open a bunch of issues that we can handle going forwards that were mentioned in this PR. Thanks so much for putting some love into the website! I think it was much needed. :)

@conradsnicta
Copy link

new website has link to old mlpack: mlpack/mlpack#2109

@rcurtin
Copy link
Member

rcurtin commented Dec 7, 2019

Alright, the initial deployment is done and I've worked out all the bugs. The master branch here on Github is now the same as what's on my system. I'll now open issues from the discussions we had here that we can address individually going forward. 👍 Thanks again @gmanlan for the hard work here! Really happy to have it deployed.

@gmanlan
Copy link
Contributor Author

gmanlan commented Dec 10, 2019

@rcurtin: thanks for all your help. This is fantastic and I'm happy to see it live!

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.

6 participants