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

feat(pipelines): ✨ Config allowed pipeline branches #500

Open
wants to merge 6 commits into
base: next
Choose a base branch
from

Conversation

JWWilks
Copy link
Contributor

@JWWilks JWWilks commented Jul 11, 2024

🚨 Proposed changes

Adds the option for an entity to configure a a whitelist of "refs" which will be shown in the Gitlab Pipelines table. If not configured, all refs/branches will show by default.

EG: gitlab.com/pipeline-refs: 'main,develop,feature/*'

Feedback from stakeholders I've worked with has been that the table is too cluttered, and that they just want to see an overview of the pipelines on the most important branches. (Probably because they don't want to see "failed, failed failed" for pipelines which run on MRs and testing branches). Obviously you can use the table filter to drill down but:

  1. The initial default view is important to adopters I've worked with.
  2. The text search filter on the table searches all fields, which can result in false results. (Eg: I search for "main" and get "feat/add-main-menu".

An alternative solution to this would be to have a [x] Show Default Branch checkbox under the card title which is defaulted to checked, or maybe [x] Show Relevant Branches which would show the default branch + ^dev(elop(ment)?)?"$. I'm happy to contribute something like that instead if you'd prefer.

⚙️ Types of changes

What types of changes does your code introduce? Put an x in the boxes that apply

  • New feature (non-breaking change which adds functionality)
  • Bugfix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)
  • Refactor

@JWWilks
Copy link
Contributor Author

JWWilks commented Jul 11, 2024

I've just realised that I can extend getPipelineSummary on my implementation side to achieve a similar result, but will leave the PR open anyway in case you're interested in these changes.

Plus, it's a little nicer this way because extending getPipelineSummary doesn't give me access to the entity annotation data.

@antoniomuso antoniomuso changed the base branch from main to next July 17, 2024 07:30
Copy link
Contributor

@antoniomuso antoniomuso left a comment

Choose a reason for hiding this comment

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

Hi, thank you for the pull request. This is a nice feature! There is only one thing that should be fixed.

if (pipelineObjects && projectObj) {
pipelineObjects.forEach((element) => {
element.project_name = projectObj.name;
});
}
return pipelineObjects || undefined;

const relevantPipelineObjects = refList
Copy link
Contributor

Choose a reason for hiding this comment

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

Doing this can reduce the number of pipelines you get:

Example

Having this annotation:

gitlab.com/pipeline-refs: 'main,develop'

if in your last 50 pipelines, 49 are in branches different from main and develop you will get only one pipeline in your card. Then you can make a request for each branch ex. /projects/2416/pipelines?ref=${branch} (with this approach you get more pipelines than before) or better you can use graphql

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @antoniomuso, good catch haven't forgotten about this - just swamped with other work. Hoping to pick this back up in early aug :)

@JWWilks
Copy link
Contributor Author

JWWilks commented Aug 7, 2024

Made some changes, this was the best I could come up with while maintaining the wildcard functionality I added.

let page = 1;
let response;
do {
response = await this.callApi<PipelineSchema[]>(
Copy link
Contributor

Choose a reason for hiding this comment

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

You cannot crawl all pipelines because it can be very inefficient. I suggest using branches API, to get all eligible branches and then query the pipelines API with the right branches.

Copy link
Contributor

@antoniomuso antoniomuso Aug 8, 2024

Choose a reason for hiding this comment

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

'Branches' API has the qs parameter search that is very useful to implement the wildcard.

Copy link
Contributor Author

@JWWilks JWWilks Aug 8, 2024

Choose a reason for hiding this comment

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

Can do. How do you recommend configuring the plugin so that you can test the backend against a real GitLab instance locally? I've been running yarn start which only tests with the mock response with mock queries.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it is hard to test with a real GitLab instance; I have to work on it to increase DevEx. I usually test it by integrating it into a pre-configured backstage environment configured with gitlab.com and I usually link the library using yarn.

Copy link

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants