Skip to content

Conversation

finestructure
Copy link
Member

@finestructure finestructure commented Oct 27, 2024

CleanShot 2024-10-27 at 10 27 17@2x

To do:

  • add test
  • check query performance

@cla-bot cla-bot bot added the cla-signed label Oct 27, 2024
.forEach(customCollections, { collection in
.a(
.href(collection.url), // FIXME: link to custom collection page
.href(SiteURL.collections(.value(collection.name)).relativeURL()),
Copy link
Member Author

@finestructure finestructure Oct 27, 2024

Choose a reason for hiding this comment

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

Using the name to build the URL works fine but generates URLs like

http://localhost:8080/collections/SSWG%20Graduated

We could make the slug configurable in the custom-package-collections.json by adding another field but I've kept it simple for now:

    {
        "name": "SSWG Graduated",
        "description": "SSWG packages that are in 'graduated' state",
        "badge": "SSWG",
        "url": "https://raw.githubusercontent.com/finestructure/sswg-package-lists/refs/heads/main/graduated.json",
        "slug": "sswg-graduated" // or maybe "id": "sswg-graduated"
    },

@finestructure
Copy link
Member Author

The query cost for CustomCollectionsController.query is ~13k, which is OK (plus we only run the query when the actual page is shown).

Limit  (cost=13491.68..13491.71 rows=10 width=3423) (actual time=69.997..70.002 rows=7 loops=1)

The additional query cost for the collections fetch in PackageController.GetRoute

            async let customCollections = customCollections(on: database, package: packageResult.package)

is minuscule cost-wise (~2) but of course brings some extra latency because the additional query needs to be run.

@finestructure finestructure marked this pull request as ready for review October 27, 2024 10:29
@finestructure finestructure force-pushed the custom-collection-page branch 2 times, most recently from e955e85 to 357a813 Compare October 28, 2024 15:19
Copy link
Member

@daveverwer daveverwer left a comment

Choose a reason for hiding this comment

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

Just a couple of points of general feedback:

  1. We’re using the term “collections” for these but we use “collections” for package collections elsewhere on the site. I don’t dislike the name, but do think it’s confusing as it stands. Off the top of my head, I could think of a couple of a couple of other terms that might work: “Package Suite” might work, “Package Group” is dull but might work, “Package Set” also might work.
  2. There should probably be a package collection for them at some point, but even if each “collection” had a package collectionI don’t think it fixes the issue with the naming.
  3. We should echo the description out under the title and make sure that markdown links are parsed in the description. I can see the SSWG wanting to link back to their page on Swift.org.
  4. We should also add another paragraph under the description that explains what this is. Something like: “Package collections/suites/groups/sets are human-curated lists of packages controlled by groups affiliated with Swift.org. Find out more in the [launch blog post].”

@finestructure
Copy link
Member Author

Regarding 1+2: I think of them exactly as package collections - it's a list of packages - and in fact I was planning to add the same package collection link we have on the author's page in a follow up :)

Re 4: We've done this a couple of times (link to launch posts) but I wonder if we should rather create a more "timeless" page explaining things than blog posts. We don't typically go back and update them when things change. With the .spi.yml we now have the SPIManifest documentation to link to but a simple documentation page (not DocC, just a markdown page) would be better than a blog post I think.

I'd suggest we leave that to a follow up where we create that page and add the link back from the list.

@finestructure finestructure marked this pull request as draft October 29, 2024 07:46
@daveverwer
Copy link
Member

Apologies, I didn't realise I had left this unapproved.

Regarding 1+2: I think of them exactly as package collections - it's a list of packages - and in fact I was planning to add the same package collection link we have on the author's page in a follow up :)

I think that works 👍

Re 4: We've done this a couple of times (link to launch posts) but I wonder if we should rather create a more "timeless" page explaining things than blog posts. We don't typically go back and update them when things change. With the .spi.yml we now have the SPIManifest documentation to link to but a simple documentation page (not DocC, just a markdown page) would be better than a blog post I think.

I'm all in favour of this. We will want to do a blog post about this so I was just trying to reduce work, but we can do both.

@finestructure finestructure force-pushed the custom-collection-page branch from 357a813 to f2fd092 Compare October 31, 2024 13:50
@finestructure
Copy link
Member Author

I've now added the package collection link + button to the collection page:

CleanShot 2024-11-01 at 10 50 06@2x

@finestructure finestructure marked this pull request as ready for review November 1, 2024 10:30
@finestructure finestructure force-pushed the custom-collection-page branch from 93f60fd to 3e0e3e8 Compare November 1, 2024 10:30
@finestructure finestructure merged commit 807708c into main Nov 1, 2024
6 checks passed
@finestructure finestructure deleted the custom-collection-page branch November 1, 2024 10:51
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.

2 participants