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: allow for exemption of repos from dependency graph integrator #1385

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tjsilver
Copy link
Contributor

@tjsilver tjsilver commented Jan 9, 2025

What does this change?

  • Repocop collects the github_repositories_custom_properties table.
  • Adds a check for the custom property gu_dependency_graph_integrator_ignore and excludes any repos that have this property from dependency graph integration.
  • Updates the tests and adds a couple more for the exemptions.

Why?

There is at least one repo that we know of that contains a script written in Scala file but that doesn't use sbt. As there are no dependencies for Scala in the repo, it doesn't need to have the sbt dependency submission workflow. Currently, the dependency graph integrator will identify this repo as requiring a workflow, and continue to raise a PR against it if the existing PR is closed or if the workflow is not merged in.

How has it been verified?

  • Added the custom property to the repo in question and observed the custom property appear in the Cloudquery table github_repositories_custom_properties.
  • Tested locally
  • Tested on CODE

@tjsilver tjsilver requested review from a team as code owners January 9, 2025 09:46
@tjsilver tjsilver force-pushed the ts/custom-props-dep-graph-ignore branch from 155b68a to 8b71e13 Compare January 9, 2025 10:21
@tjsilver tjsilver force-pushed the ts/custom-props-dep-graph-ignore branch from 8b71e13 to 07ce2f6 Compare January 9, 2025 10:34
@tjsilver tjsilver force-pushed the ts/custom-props-dep-graph-ignore branch from 07ce2f6 to 92dc4d4 Compare January 9, 2025 10:36
@@ -119,10 +121,27 @@ async function sendOneRepoToDepGraphIntegrator(
}
}

export function getReposWithoutWorkflows(
export function repoIsExempted(
Copy link
Member

Choose a reason for hiding this comment

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

Should the language be passed in here? ie. 'Scala' or 'Kotlin'

Comment on lines +128 to +129
const repoIsExcepted = exceptedCustomProperties.find(
(property) => repo.id === property.repository_id,
Copy link
Member

Choose a reason for hiding this comment

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

Should the value of the property be compared to the language here? eg. is it Scala

value: 'Scala',
};
}

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we need another test for Kotlin and one for an entirely irrelevant language to prove that we distinguish

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

Successfully merging this pull request may close these issues.

2 participants