Skip to content

feat: add plugin to fetch terasology engine info from github#197

Merged
jdrueckert merged 6 commits intomasterfrom
feat/plugin
Jan 14, 2023
Merged

feat: add plugin to fetch terasology engine info from github#197
jdrueckert merged 6 commits intomasterfrom
feat/plugin

Conversation

@jdrueckert
Copy link
Copy Markdown
Member

  • adjust .gitignore
  • add workspace
  • consume plugin

@jdrueckert jdrueckert added the enhancement New feature or request label Jan 12, 2023
@jdrueckert jdrueckert requested a review from skaldarnar January 12, 2023 17:27
Copy link
Copy Markdown
Member

@skaldarnar skaldarnar left a comment

Choose a reason for hiding this comment

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

We can move forward merging this to unblock you working on the module and engine pages with good first issues listed.

But in the long run, I think we should only have a single custom source plugin and just fetch information for both the engine and modules.

query Engine {
organization(login: "MovingBlocks") {
repository(name: "Terasology") {
issues(first: 10, filterBy: {labels: "Good First Issue", states: OPEN}, orderBy: {field: UPDATED_AT, direction: DESC}) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are we only fetching the first ten issues here? This seems to be quite a few, and even if we fetch more we can still decide to only display a few on the page...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Let's change this to fetching all of them with respective pagination in a follow-up PR

Comment on lines +12 to +25
nodes {
id
title
author {
login
}
labels(first: 10) {
nodes {
name
}
}
updatedAt
url
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is (naturally) the same structure as we use for fetching issues for modules. We could use GraphQL Fragments to make sure they always have the same structure.

Which makes me wonder whether this should actually be a separate plugin, or whether we just merge it with the other one for simplicity (and easier sharing of code and query fragments....)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

let's consider doing that in a follow-up PR 👍

Comment on lines +51 to +52
const lastFetchedKey = "terasology-engine-last-fetched";
const dataKey = "terasology-engine-data";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the caches are scoped for the plugins individually, so prefixing is not strictly required. This applies to the other plugin as well, so that's rather a note to self here...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

guess if we plan to merge the plugins anyway, we can make use of the prefix for scoping then, so keeping it for now.

@sonarqubecloud
Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jdrueckert jdrueckert merged commit 5ff6aad into master Jan 14, 2023
@jdrueckert jdrueckert deleted the feat/plugin branch January 14, 2023 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants