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 new repositories fragment #803

Merged
merged 5 commits into from
Jul 22, 2020

Conversation

mpourismaiel
Copy link
Member

@mpourismaiel mpourismaiel commented Jul 18, 2020

What this PR does / why we need it:
Added new fragment called "repositories". Demo and documentation is available at /fragments/repositories. Currently it only supports Github. I plan to add Bitbucket and Gitlab shortly but adding them would require a simple interface to keep us from writing the template multiple times. Writing the code isn't that hard but I prefer to get approval on the design and how it's currently implemented before I move on.

image

Which issue this PR fixes:
fixes #599

Release note:

- repositories: Added new fragment to fetch and display user repositories from Github

@mpourismaiel mpourismaiel changed the title WIP: Add new repositories fragment Add new repositories fragment Jul 20, 2020
@mpourismaiel mpourismaiel self-assigned this Jul 20, 2020
@mpourismaiel mpourismaiel requested a review from stp-ip July 20, 2020 13:31
@mpourismaiel mpourismaiel added this to the v0.18.0 milestone Jul 20, 2020
Copy link
Member

@stp-ip stp-ip left a comment

Choose a reason for hiding this comment

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

Overall not sure if repositories is the best naming choice especially considering we want to switch to provider specific fragments. Should we start with this fragment and name it "repositories_github".

Meaning we would have "/_".
Examples: "tailwind/contact", "contact", "contact_netlify"

I like the simplicity of it pulling in the latest repos from a user. One downside would be, that it would also pull in forks and place them at the top of the page.
Not sure on it being an issue or how to solve it. First idea would be a whitelist. Other data could still be pulled in from Github.

Another option that could also be a separate fragment could be a similar style, but with sub item config configuring git url, repository and project page etc.

With the API usage we could add stars, forks, contributors, issue url, release url etc. automatically in a follow on PR I think.

Comment on lines +230 to +241

.repositories {
.repository {
display: flex;
justify-content: stretch;
align-items: stretch;

.card {
flex: 1;
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Why add the repositories styles to the global syna config?

*type: number*
*default: updated_at*

The variable name to use for sorting repositories.
Copy link
Member

Choose a reason for hiding this comment

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

Why "variable name"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can't think of a good explanation, not happy with it either. Any suggestions?

The number of repositories that will be rendered.

#### sort
*type: number*
Copy link
Member

Choose a reason for hiding this comment

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

Why is this a number?

Copy link
Member Author

Choose a reason for hiding this comment

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

Totally my bad.


#### sort
*type: number*
*default: updated_at*
Copy link
Member

Choose a reason for hiding this comment

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

What options are available? Star count? Contributors?

<div class="alert alert-primary" role="alert">
{{% md %}}
For now, we only support Github. You can review the [documentation for their API](https://docs.github.com/en/rest/reference/repos#list-repositories-for-a-user).
We will add support for more sevices in the future.
Copy link
Member

Choose a reason for hiding this comment

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

Not with the current configuration adding more services would be possible. It feels quite focussed on github. Adding other services would need more data from the user such as sub items with link to project on say gitlab or github.

We could leave it this simple and make it a first provider fragment: "repositories_github" for example.

Copy link
Member Author

Choose a reason for hiding this comment

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

So rename the fragment to repositories_github and remove the alert?

I was thinking about adding a provider and framework (or variation) variables instead of naming convention. It's not related to this issue though, just wanted to mention it somewhere since I tend to forget a lot :D

+++
fragment = "repositories"
weight = 100
user = "octocat"
Copy link
Member

Choose a reason for hiding this comment

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

We should probably showcase "okkur" for good measure :)

<div class="card">
<div class="card-body">
<h5 class="card-title">{{ .name }}</h5>
<h6 class="card-subtitle mb-3 text-muted">{{ printf "Last updated on: %s" (dateFormat "02/01/2006" .updated_at) }}</h6>
Copy link
Member

Choose a reason for hiding this comment

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

Why not use the ISO date format aka "2000-12-31"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I somehow forgot to use the standard format :D Will update it.

@mpourismaiel
Copy link
Member Author

I don't think subitems would be a good idea for this fragment. A whitelist of repositories can be created using a variable, specifying fields is something that is applied on all the cards (I don't think changing the fields per project is a good idea, both design-vise and the confusion that it causes to the end user). Also we can allow a filter function to be written by the user and call that. For example, we write our sort and filter functionality in a helper partials/helpers/repositories_github/fitler.html that returns a filtered list and document how to write new filters. We can keep the sort and count in our code for the sake of simplicity as well.

@stp-ip
Copy link
Member

stp-ip commented Jul 22, 2020

Ok so we keep it simple.
Make it the first provider specific fragment.
Adding more buttons from the data as follow on.

So default sorting is just by last commit date.
We should have at least star count as well. Sidequestion there. Does a fork have the same star count as the main repo? If not, then filtering wouldn't be needed. We would just have star count default sort and it would always bubble up the users personal repos and show the 5 most important or so.

Not sure about what you are imagining on the filter side. Care to elaborate? My first thought was just whitelisting of repos as a list.

Thinking about lists. Any reason to not make the "user" not a list? Also is this better to be called "user" or namespace?
It being a list of namespaces for example we could do something like: "okkur, okkur-incubator".

The other option there is to keep it simple and pull in only one namespace/user and the Syna user could always create multiple separate fragments.

@mpourismaiel
Copy link
Member Author

Sorting and filtering is a bit of a problem since we can't get pinned repositories and have to rely on sorting and limiting or filtering (based on a whitelist or "no forks" somehow) and again, limiting with count. We can however extract the code we use to filter repositories into a helper file that receives repos as a parameter and returns the new list. Then any user who wishes to change the list to their liking can override that helper, copy it to their website and update its code.

@stp-ip
Copy link
Member

stp-ip commented Jul 22, 2020

I would leave advanced sorting and filtering for now, but add these points:

  • sorting by date (what we have)
  • sorting by star count (new default)
  • forks true/false (false by default)
  • whitelist that only shows repos that match the whitelist (not set by default)

Happy to do the whitelist idea in a separate PR. The others I feel are good things to have here already. Thoughts?

@mpourismaiel
Copy link
Member Author

mpourismaiel commented Jul 22, 2020

image

@stp-ip Also updated fragment's name and changed forks to display_forks. I think it's more intuitive.


**Note:** The variable name is a key in the response of Github's repositories API. Also the sort is done using Hugo's `sort` function.
**Note:** This value is a key from the response of Github's repositories API. Also the sort is done using Hugo's `sort` function.
Copy link
Member

Choose a reason for hiding this comment

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

So can be any key from the Github response. Nice.

Copy link
Member

@stp-ip stp-ip left a comment

Choose a reason for hiding this comment

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

/lgtm

@stp-ip
Copy link
Member

stp-ip commented Jul 22, 2020

Let's do the whitelist and maybe additional buttons in a separate PR.

@stp-ip stp-ip merged commit f5938c9 into okkur:master Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

[cards-repo] Add repo cards for items
2 participants