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

Add a simple website #67

Closed
wants to merge 39 commits into from
Closed

Add a simple website #67

wants to merge 39 commits into from

Conversation

davorg
Copy link
Contributor

@davorg davorg commented Jan 13, 2025

@book asked me to look at using GitHub Actions/Pages to put a simple website in front of this repo. This is what I've knocked up in the last couple of hours. It's not particularly pretty, but I figure it's a start and now we have the skeleton in place, we can iterate on it to make improvements.

If you decide to merge this, there's a bit of GitHub Pages admin needed. I'm happy to talk you through that or do it myself if you give me appropriate permissions on this repo.

You'll need to decide on a domain to host it (or you can use the GitHub default which would (I think) be https://perl.github.io/PPCs. In the meantime, my fork is deployed at https://ppc.perlhacks.com/ https://davorg.dev/PPCs/ (moved it) for you to look at.

@jkeenan
Copy link

jkeenan commented Jan 13, 2025

The Status column should have definitions for the various statuses. I wondered, "Does 'Implemented' mean that the PPC has appeared in a production release? If so, which one (with date)?"

Otherwise, I appreciate the simplicity of the design.

@davorg
Copy link
Contributor Author

davorg commented Jan 13, 2025

As I said, this is just a skeleton - more of a proof of concept. There are plenty of things that the PSC could add

@guest20
Copy link

guest20 commented Jan 14, 2025

This is the greatest Perl Proposed Change Page (ppcp?) I've ever seen! Looks good.

I especially like that it doesn't have a popup about disabling ad blockers, accepting cookies, or a thing about subscribing to a mailing list.

  • Maybe put the json link at the bottom and add a quick description of what a PPC is in its place on the front page?
  • Is it worth masking email addresses somehow?
  • I think the PPC's title should be the first non-ID column in the list of ppcs (especially if the elements re-order in a responsive way for telephone-based viewing)
  • Have you considered sticking the new camel on it? https://github.com/metacpan/perl-assets/blob/main/blessed/src/080.svg
  • I think the PPC template is more useful if it's not rendered to html, since it's there to be copied into a file in git. It might benefit from a js "copy all this text" button.

Place sticky footer content here.

The footer does not seem to be sticky'ing.

As I said, this is just a skeleton

spooky

@davorg
Copy link
Contributor Author

davorg commented Jan 14, 2025

@guest20 The content is for the PSC to think about, My mission here was to demonstrate how it might work - which I think I've done.

@haarg
Copy link
Contributor

haarg commented Jan 14, 2025

I think putting this at https://perl.github.io/PPCs will be fine.

@guest20
Copy link

guest20 commented Jan 14, 2025

@davorg In that case I'm pleased to report that your PR has definitely convinced me that the internet is a suitable vehicle for templated hypertext documents.

@Grinnz
Copy link

Grinnz commented Jan 16, 2025

I think that this is a great idea for visibility into the process and the decisions being made, and I agree that the default github.io URL serves it well as a self-documenting indication of where the PPCs are maintained.

I would suggest moving the "Title" column to directly after the PPC number, to make it easier to scan for their names. (which I see that @guest20 has also suggested)

@book
Copy link
Contributor

book commented Jan 16, 2025

As I said, this is just a skeleton - more of a proof of concept. There are plenty of things that the PSC could add

And thank you very much for this.

I intend to spend some time playing with your proof of concept to organize the documents in a way that is more suitable to the process: the most important documents are the ones that are still being reviewed, or still being implemented. The PPCs for proposals that have either been rejected or added to Perl only serve as historical reference.

@rwp0
Copy link
Contributor

rwp0 commented Jan 17, 2025

Thanks, three minor thoughts/suggestions while checking the design of the fork:

image

  1. Unlink "as" in "as JSON" since "JSON" is enough already.

  2. @XXXX hurt the eyes, remove the emails altogether instead of hiding.

  3. Maybe remove underscores from Title's since it may also affect readability negatively.

Also, I wish the columns were sortable so we can easily check what's Implemented for instance.

Again, thank you very much for this is a worthy addition/integration IMHO.

@esabol
Copy link

esabol commented Jan 24, 2025

I think this is definitely needed, and it's a wonderful contribution.

Minor nitpicks of the web page itself:

  1. Add a period after "as JSON".
  2. Change "Github" to "GitHub".

@davorg
Copy link
Contributor Author

davorg commented Jan 24, 2025

I've been quietly making some small improvements. A lot of the suggestions I've seen here have already been implemented. See the various commit titles for details.

And just now, I've added simple Datatable support, so the table on the front page can be paginated, sorted and filtered.

@karenetheridge
Copy link
Member

There are no timestamps.. I think the most relevant timestamp would probably be the date of last comment/commit/update, which surely is available in the github pull request metadata somewhere?

@guest20
Copy link

guest20 commented Jan 25, 2025

@karenetheridge

PR info isn't going to be a great choice here:

  • You don't want to trigger a build every time somebody comments, so you'll end up with last-comment dates from the most recent merge
  • PRs/Comments aren't in git, so either the site, or the build need to query github's graphql api

The question is "what are you looking to rank up?"... maybe sorting on Status first (to put the most sensational new ideas at the top), then on commit date (to keep the newest top'est) would make the most sense?

@guest20
Copy link

guest20 commented Jan 25, 2025

"0006 	Module Loading with "load_module""

That \ in there makes me feel like there's some kind of (possibly unsafe) markdown shenanigans going on, obviously the _ needs to be escaped in markdown so it's not turned into an <em>, but the list on the home page is supposed to be rendered markdown, not markdown source.

(I just fixed an unrelated bug in unrelated code where my table-of-contents logic contained literal [](http://...) because it forgot to parse headings for links before putting them in the ToC so I might be being paranoid here)

ttlib/page.tt Outdated
<meta name="viewport" content="width=device-width, initial-scale=1">
<meta name="description" content="">
<meta name="author" content="Mark Otto, Jacob Thornton, and Bootstrap contributors">
<meta name="generator" content="Hugo 0.84.0">

Choose a reason for hiding this comment

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

Cargo culted? We're not using Hugo here.

Copy link

Choose a reason for hiding this comment

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

Maybe it's a security measure to distract vulnerability scanners?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'll fix that. It's there because I copied the code from a Bootstrap example site.

I really wouldn't describe it as cargo-culting - I know exactly what all of those lines do 😊

lib/PPC.pm Outdated
@@ -0,0 +1,86 @@
use experimental qw[builtin class signatures];
Copy link

Choose a reason for hiding this comment

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

Maybe add a use v5.38 at the top, like the build script, I don't think anything in this file currently turns on strictures/warnings. As a benefit the the 5.38 bundle has module_true so you can drop the 1; from the end of this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion. Thanks. Done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also fixed the warning that this revealed :-/

Copy link

Choose a reason for hiding this comment

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

Nice, while we're here is there any reason not to use the latest Perl, 5.40? Looks like the GH Action uses "latest".

Using 5.40 would present a few opportunities to remove boilerplate:

Builtin is not longer experimental, so we can just do:

use experimental qw[class signatures];

Builtin has version bundles too now which are automatically imported by the corresponding use version line so we can drop this line completely:

use builtin 'trim';

And the biggest win is we can apply :reader to our fields and drop our handed rolled readers:

field $author  :param :reader = '';
field $id      :param :reader;
field $slug    :param :reader;
field $sponsor :param :reader = '';
field $status  :param :reader;
field $title   :param :reader;

Remove:

method author  { return $author }
method id      { return $id }
method slug    { return $slug }
method sponsor { return $sponsor }
method status  { return $status }
method title  { return $title }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is there any reason not to use the latest Perl, 5.40?

Honestly, just laziness. The system I was working on had 5.38 installed :-)

But, yeah, I should update that.

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

@davorg
Copy link
Contributor Author

davorg commented Feb 5, 2025

@Perl/perl-steering-council Is there anything else you need before merging this?

@book
Copy link
Contributor

book commented Feb 6, 2025

Thanks, I rebased your branch, and applied it as 1d5e7a4.

@book book closed this Feb 6, 2025
@esabol
Copy link

esabol commented Feb 6, 2025

Thanks, I rebased your branch, and applied it as 1d5e7a4.

The URL (https://perl.github.io/PPCs) for this website probably needs to be added to https://github.com/Perl/PPCs/blob/main/README.md. I also recommend putting it in the "About" box of the project on GitHub.

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.