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

preview implementation of the Laminas ecosystem RFC #226

Open
wants to merge 8 commits into
base: staging
Choose a base branch
from

Conversation

Jurj-Bogdan
Copy link
Contributor

Q A
New Feature yes
RFC yes

Description

This PR is an initial implementation for the Laminas ecosystem RFC. There are still features I think should be implemented (i'll list a few at the end), but hopefully there's enough for a preview to help continue the conversation started in the RFC.

Current functionality

The laminas ecosystem:create-db and laminas ecosystem:seed-db commands are used to generate and update the list found on '/ecosystem' by using the user-provided data from ecosystem-packages.json.

As the consensus from the RFC is that packages must be installable via composer, the Packagist url is mandatory and used to make a single request per package, from which the displayed data is extracted.
If provided, the 'homepage' and 'githubUrl' data will be used to overwrite the data coming from Packagist, and the 'categories' are used as extra filtering options in the package listing.

The create-db command currently works similar to the blog version, regenerating the whole database file after each build, although i would change this to make it more future proof. I've thought of the seed-db command as a cron used to select packages not updated in X hours and update some of the relevant data - would something real-time (or closer to) be desired instead?

As for the listing page ('/ecosystem') I've left a low page size to make it easier to test the pagination with the few default packages. For now there's searching by package name and filtering by "tags" (keywords taken from packagist) and "categories" defined by the user in the json file.

what next?

  • i'm aware of the failing psalm check, but seeing as this PR is subject to change, I'll make sure to clean that up once i have a better understanding of what is required from this feature
  • maybe adding a github action to validate the ecosystem-packages.json file, to make it easier for the reviewer ?
  • if the current approach is kept, i think changing the database creation command to only add the newest entries if the db already exists would be a good change
  • design changes
  • as both the blog search and maintenance status features provide json responses, this could be added to the ecosystem package list as well if desired
  • expanding the instructions for adding a new package once the desired rules and approach is agreed on

@froschdesign
Copy link
Member

@Jurj-Bogdan
What was the link to the preview?

@arhimede
Copy link
Member

arhimede commented Nov 1, 2024

@Jurj-Bogdan What was the link to the preview?

Is not built yet, I need to do some git operations first

{
"packagistUrl": "https://packagist.org/packages/netglue/laminas-messenger",
"githubUrl": "https://github.com/netglue/laminas-messenger",
"categories": ["laminas", "messenger"],
Copy link
Member

Choose a reason for hiding this comment

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

"laminas" does not make sense here, because we need to differentiate between integration in Mezzio-based and/or laminas-mvc-based applications. This means that the package can be used as a:

  • module in a laminas-mvc-based application
  • and/or via a config provider in a Mezzio-based application

Maybe the categories are not suitable for this and a separate entry or entries are required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment the "categories" key is only used for filtering in the interface, so I added a few here and there so they'll be visible in the preview - once a new role is found for them, I can upgrade the functionality to fit the request of course.

Even if the current approach of "user defined keywords" is kept, some other guidelines could be defined for them if wanted.

Copy link
Member

Choose a reason for hiding this comment

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

we need to brainstorm a bit about those possible categories.

@arhimede
Copy link
Member

arhimede commented Nov 1, 2024

PREVIEW HERE:
https://preview-1-hy2vwsq-2ja7ciew2nbkm.us-2.platformsh.site/ecosystem/

@froschdesign
Copy link
Member

froschdesign commented Nov 1, 2024

My suggestions:

  1. Use Bootstrap's cards and create 3 columns for large screens.
  2. Maybe an image can be defined for a package entry, based on the Bootstrap icons or using the social media preview of a repository (Unfortunately I don't know if you can retrieve this from API.).
  3. Remove the paginator.
  4. Reduce the information of a package. For example, the number of watchers is not really necessary.

@Jurj-Bogdan
Copy link
Contributor Author

A few details and questions about the latest commit:

  1. The packagist api doesn't give access to the repository's social preview, so I've added a separate call to github's graphql api to only bring the image URL. A few issues here:

    • if a repository has no custom social preview, an image will be generated for it which will double some already available info (name, number of stars, example below), but I can't remove said information as I don't have a way to know what info is displayed in the preview (i.e. for a custom preview such as dotkernel api's that doesn't provide a name etc.)
      • to note is that the packagist api provides access to the repository owner's avatar, so if wanted this avatar could be used instead, so the separate github api call can be removed - however this means custom social previews would be ignored, it's your call
      • another option would be to add an optional "image" key to the json file, where the user would manually submit a URL to be displayed
        image
        image
  2. Is this version of getting data from the api ("create-db" command run on build + "seed-db" command used as a cron for data refreshing every X hours) alright with you or would another way be desired? If it's OK i'd like to (and should) go ahead and clean up the code alongside any future changes.

    • In the initial commit, the "create ecosystem db" command worked similar to the blog one by deleting the existing db file and regenerating in on each build which will be problematic with large numbers of packages added. I've changed this so the existing database is kept and packages existing in the db are skipped when getting data from the api. This way will also ensure different "created/updated" dates for the cron to update packages in smaller batches.
  3. about the data saved and displayed for each package:

    • i've kept the keywords taken from the package's composer.json and the user-defined keywords from the ecosystem-packages.json and am basically using them as filters, however they can clutter up the card in large numbers - do you think any of them should be removed/limited in number?
    • the Composer package type is now pulled from the API and displayed as it feels relevant, would filtering by type be a good addition?
    • similar to the above, i've added a user-defined "category" with the "skeleton", "integration" (or "wrapper" as mentioned in the original issue) and "tool" values available, which again seem to be a useful information to display and maybe to filter based on the intended use case - should I go ahead with this feature?

@arhimede
Copy link
Member

@arhimede
Copy link
Member

now the social preview is not looking good
image

We will try a variant to use as image or "social preview" only the github organisation logo .

@gsteel
Copy link
Member

gsteel commented Nov 19, 2024

My 2 cents:

  1. I think that using the GH social preview is probably the best plan - user avatars are a bit hit and miss and also less relevant - For example, the avatar for all the laminas packages is @weierophinney gravatar:
    Mwops avatar
    Can we get away with no image at all as an alternative or does the layout get too boring? Also, what about libs hosted on GitLab/Codeberg etc?
  2. Retaining the DB seems like a sane plan 👍
  3. Usage of the composer package type is really worth-while - I don't think the concept of "type" should be extended beyond the ones composer offers, that said "category" is helpful, but I believe these should be small in number, predefined and packages should only appear in one category. The main problem I see here is: lots of "integrations" and a handful of "tools" and "skeletons" at which point category starts feeling less useful.
    I think that the main filtering should be done via user-supplied tags that are audited by us prior to merge, rather than relying on GH topics or composer keywords - this will be the main tool that will allow us to curate collections so that they are meaningful for users.

@gsteel
Copy link
Member

gsteel commented Nov 19, 2024

Also, rather than "See on GitHub", perhaps just "Source" with the icon for the relevant source code hosting platform?

@weierophinney
Copy link
Member

How are you/will you determine packages to include? I ask, because, for instance, the phly-simple-page package on there likely should not be (I've not updated it in 3 years, and am in fact planning on archiving all my ZF2 packages (even if I did update them for Laminas at some point).

@arhimede
Copy link
Member

How are you/will you determine packages to include? I ask, because, for instance, the phly-simple-page package on there likely should not be (I've not updated it in 3 years, and am in fact planning on archiving all my ZF2 packages (even if I did update them for Laminas at some point).

All packages we have listed now are only test data.
In order to decide the layout and the logic of that page.
Will be subject of a future TSC meeting to decide the rules to include and remove a package from that Ecosystem page

@froschdesign
Copy link
Member

  1. You're going to hate me for this, but the term "ecosystem" is wrong here, because a few modules, packages or whatever you want to call them cannot be the ecosystem of Laminas. The listed packages/modules are a part of the ecosystem, but not the entire system.
    And as an end user, I would also not search for or suspect extensions and libraries from third parties under this name.
  2. The amount of information can be greatly reduced, because licences, downloads and various links are too much. A link to the repository and - if available - to the documentation/homepage is sufficient. (Is the link to Packagist really helpful for a user? After all, it only redirects to the repository.)
  3. If we also want to have packages/modules hosted on Gitlab or other platforms, then we need to rethink the information gathering process.
  4. I would also like to point out again that we need a clear distinction between what can be used in an MVC application and what can be used in Mezzio.
    For example, LmcUser must be listed and it must be clear to the end user that this only works in a laminas-mvc based application.

@froschdesign
Copy link
Member

A consistent layout would also be desirable, and the unnecessary spacing due to paragraphs is not necessary.

preview-packages

The description text at the end, then the problems are reduced. Example: https://getgrav.org/downloads/plugins

@arhimede
Copy link
Member

  1. You're going to hate me for this, but the term "ecosystem" is wrong here, because a few modules, packages or whatever you want to call them cannot be the ecosystem of Laminas. The listed packages/modules are a part of the ecosystem, but not the entire system.
    And as an end user, I would also not search for or suspect extensions and libraries from third parties under this name.

We can call it "directory" instead , even the word "ecosystem" is more catchy :-)
This will be a subject of a vote in TSC.

  1. The amount of information can be greatly reduced, because licences, downloads and various links are too much. A link to the repository and - if available - to the documentation/homepage is sufficient. (Is the link to Packagist really helpful for a user?
    After all, it only redirects to the repository.)

Agree, now it is too much information.
@Jurj-Bogdan
One nice directory is the one for React Directory

  1. If we also want to have packages/modules hosted on Gitlab or other platforms, then we need to rethink the information gathering process.

The information will be gathered only from Packagist, as I presume a mandatory condition for a package to be lsited here will be to be installable using Composer.

  1. I would also like to point out again that we need a clear distinction between what can be used in an MVC application and what can be used in Mezzio.
    For example, LmcUser must be listed and it must be clear to the end user that this only works in a laminas-mvc based application.

@Jurj-Bogdan we need a Flag here, separate from "category".
MVC, Middleware, Mezzio, will see which one will be the default flags

@froschdesign
Copy link
Member

froschdesign commented Nov 20, 2024

By the way, this is boring: https://symfony.com/bundles 😉

One nice directory is the one for React Directory

Ah, this was the original inspiration for the layout.

@Jurj-Bogdan
Copy link
Contributor Author

I have started tweaking the info such as the keywords from composer being taken out, but keeping the user defined ones (probably going to limit those to 5? as well). Will also provide a sort of flag as @arhimede mentioned, with preset values which should be used as filters as well (thinking of adding a navbar around the search input for these "preset filters" - category, type and the new "flag")

Thanks @froschdesign for the example, i'll work on a version with the social preview working similar to getgrav.org's images; that is unless @gsteel 's question of removing images altogether is preferred, of course.

@froschdesign
Copy link
Member

Another example: https://astro.build/integrations/
Also interesting what information is required: https://docs.astro.build/en/reference/publish-to-npm/#integrations-library

@gsteel
Copy link
Member

gsteel commented Nov 20, 2024

Adding relevant keywords to your composer package is a great idea - I wonder if the packagist api can handle that? We'd need a way of excluding packages too though.

@froschdesign
Copy link
Member

We'd need a way of excluding packages too though.

The packages must be added manually, not automatically! Or have I missed something?

@gsteel
Copy link
Member

gsteel commented Nov 20, 2024

I was just ruminating on being able to automate that - it's probably best to stick to the plan and add them manually as originally specified 👍

@froschdesign
Copy link
Member

Adding must be done proactively and I'm not worried about that, because some people are already waiting for an official listing.
The next step would be a new series of blog articles under the name "Package/Module of the Month", or something similar.

@arhimede
Copy link
Member

Signed-off-by: Jurj-Bogdan <[email protected]>
@Jurj-Bogdan
Copy link
Contributor Author

Jurj-Bogdan commented Nov 25, 2024

an overview for the latest commits:

  1. kept the social preview as suggested by @gsteel, with a small on-hover effect to display more information, but I have a few questions related to it.
    The image has to be relatively large for the small details to be somewhat legible at least (especially for the auto-generated social previews), but to me it feels like the cards get a bit too large, so:

    • would it be better for the social preview to hide completely on-hover as opposed to the "cut off" that is up now? I could also hide more information by default, with it only popping up on hover (hide the source, download/star numbers and the description) resulting in smaller cards
    • the "type", "usage" and "category" line is also weird, the info is relevant and simply replacing the words with icons could work, but might feel confusing about which is which i think - any preference here?
    • i feel there should be some limits to the user-defined keywords, what do you think? A limited number, maybe even a character limit per keyword?
    • one issue right now is related to packages hosted on anything other than github - for these there obviously won't be any social preview to grab with the current command, so I provided some default images (by category: skeleton, integration & tool)
      • these will also be used if there are any errors when getting the social preview, such as for phly/phly-simple-page, which has another name on github - I'll fix the way previews are brought to avoid this exact issue, but i think it can be useful for others
  2. added filters by type, usage and category:

    • "type" - values are taken from packagist: "library", "project", "metapackage", "composer-plugin". Should all these types be allowed?
    • "usage" - to provide a distinction between mvc / mezzio application, as suggested by @froschdesign and @arhimede
    • "category" - can be one of "skeleton", "integration" and "tool", let me know of any better ones
      Filters can be removed individually by clicking each again in the dropdown. The "clear" filters button will not touch the search/pagination, but only remove these 3.
  3. added a flag for abandoned packages in case it's decided these would be worth displaying - let me know if they aren't desired so I can skip them altogether if any gets submitted and approved. If so, i'll have to extend the "refresh data" cron to remove any package that gets abandoned after being accepted as well

I haven't yet changed the information gathering process, until it's decided if platforms other than GitHub should be included. Same goes to using the "ecosystem" term, I'll go ahead and change that once agreed on a replacement.

@weierophinney
Copy link
Member

A few notes:

  • I wasn't sure at first about the hover effect, until I found one where the content got truncated and wasn't fully visible until the hover. As such, I think it works.
  • If a package is marked abandoned, I don't think we should display it (mezzio-aurarouter is the example here).
  • It would be nice to be able to clear individual filters, instead of needing to clear all filters.
  • Will there be pagination at some point, or will we always show everything? What will the performance be like if we show everything?
  • We need some sort of explanation for the "Category" filtering; I'm not sure what a tool is in this context.

@Jurj-Bogdan
Copy link
Contributor Author

  • individual filters can be cleared by clicking them again in the dropdown - maybe I should add an "X" so it stands out more.
  • there's a 15 packages per page limit right now and not enough packages, but yes, there is pagination available.
  • the categories aren't really intuitive, i agree, but once agreed on a set of categories i'll make sure to explain every term in an updated ADD_ECOSYSTEM_PACKAGE.md file. As it stands right now, the categories are supposed to be something like this:
    • "skeleton" - package that allows building an entire application upon
    • "integration" - package that extends an existing package's functionality
    • "tool" - "standalone" package that provides its own functionality;
    • i should add i've set this category randomly across the existing package list so there are a few available for filtering in the preview

@froschdesign
Copy link
Member

  • but once agreed on a set of categories i'll make sure to explain every term in an updated ADD_ECOSYSTEM_PACKAGE.md file.

But this does not help the user / visitor of the website. The filter must be self-explanatory on the website. I assume that this is what @weierophinney means.

@arhimede
Copy link
Member

  • but once agreed on a set of categories i'll make sure to explain every term in an updated ADD_ECOSYSTEM_PACKAGE.md file.

But this does not help the user / visitor of the website. The filter must be self-explanatory on the website. I assume that this is what @weierophinney means.

probably the word "Tool" is not the best here. We may find another one.
I want to differentiate between those 2 types of libraries:

@arhimede
Copy link
Member

arhimede commented Dec 2, 2024

@arhimede arhimede self-requested a review December 3, 2024 11:00
@@ -0,0 +1,114 @@
'use strict';

$(document).ready(function () {
Copy link
Member

@froschdesign froschdesign Dec 10, 2024

Choose a reason for hiding this comment

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

Vanilla JavaScript is good in 2024 and it should be dropped when we move to Bootstrap 5.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough, I saw jquery was already added as a dependency and went with it, but I'll go ahead and refactor

@Jurj-Bogdan
Copy link
Contributor Author

Added an extra GH workflow to make sure the .json file being edited by users is not empty and is valid JSON, as well as making sure all the required keys are defined (should I also extend it so the values are checked as well?).

Let me know if another way to handle this would be preferred, and also of any other changes to the design and/or logic as well, of course.

</div>

<div class="btn-group">
<button id="clear-filters-button" type="button" class="btn btn-outline-danger">Clear filters <i class="bi bi-x-circle"></i></button>
Copy link
Member

Choose a reason for hiding this comment

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

The button to clear the filter should only be visible if the user has set a filter and should also reset the search. Otherwise I find the handling confusing.

@Jurj-Bogdan
Copy link
Contributor Author

Replaced jQuery with vanilla JS in the custom script, and the preview has been updated: https://preview-1-hy2vwsq-2ja7ciew2nbkm.us-2.platformsh.site/ecosystem/

I'd would need some input for a few pretty major issues:

  1. The "package must be available via Composer / Packagist" rule seems set in stone at the moment, however since Packagist allows packages hosted on GitLab, Bitbucket etc., any calls to GitHub's API for the social preview will fail. For now that will result in default images being shown

    • should I look into getting data for such repositories?
    • if not, maybe some other type of default images should be shown? Currently they're simple bootstrap icons based on category: skeleton, integration & tool
  2. do you feel the the implementation itself (commands used for data gathering, storing packages in the sqlite db etc.) requires major changes? Because if not, I'll go ahead and clean up the code and fixing the Psalm errors

    • also if this implementation stays, do you think the commands should use a package to make the API calls instead of the current native cURL?
  3. somewhat related to the previous question, once the implementation is "set" should i go ahead and add tests?

@weierophinney
Copy link
Member

Replaced jQuery with vanilla JS in the custom script, and the preview has been updated: https://preview-1-hy2vwsq-2ja7ciew2nbkm.us-2.platformsh.site/ecosystem/

I'd would need some input for a few pretty major issues:

  1. The "package must be available via Composer / Packagist" rule seems set in stone at the moment, however since Packagist allows packages hosted on GitLab, Bitbucket etc., any calls to GitHub's API for the social preview will fail. For now that will result in default images being shown

    • should I look into getting data for such repositories?
    • if not, maybe some other type of default images should be shown? Currently they're simple bootstrap icons based on category: skeleton, integration & tool

Yes - exactly this last approach. If we can't look up an image, use one based on the category.

  1. do you feel the the implementation itself (commands used for data gathering, storing packages in the sqlite db etc.) requires major changes? Because if not, I'll go ahead and clean up the code and fixing the Psalm errors

Assuming you address the default image solution, yes.

  • also if this implementation stays, do you think the commands should use a package to make the API calls instead of the current native cURL?

Since we can control the environment where the command runs, cURL is fine.

  1. somewhat related to the previous question, once the implementation is "set" should i go ahead and add tests?

Yes, please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants