Lint command #31
Lint command #31
Conversation
tools/integration/src/index.ts
Outdated
| if (!fs.existsSync(dashboardsPath) || !fs.statSync(dashboardsPath).isDirectory()) { | ||
| missingFiles.push('dashboards folder'); | ||
| } | ||
| if (!readmeContent) { |
tools/integration/src/index.ts
Outdated
| console.error('README.md file content is empty or missing.'); | ||
| missingFiles.push('README.md'); | ||
| } | ||
| if (missingFiles.length > 0) { |
tools/integration/src/index.ts
Outdated
| // Function to handle lint logic | ||
| async function handleLint(argv: any) { | ||
| const currentDirectory = process.cwd(); | ||
| console.log(`Current working directory: ${currentDirectory}`); |
There was a problem hiding this comment.
Probably we shall use logger instead of console consistently throughout the lint function.
tools/integration/src/index.ts
Outdated
| missingFiles.push('dashboards folder'); | ||
| } | ||
| if (!readmeContent) { | ||
| console.error('README.md file content is empty or missing.'); |
There was a problem hiding this comment.
Probably we can omit such log output consistently since we will output the lint results at last.
tools/integration/src/index.ts
Outdated
| } | ||
| if (missingFiles.length > 0) { | ||
| console.error(`Missing required files or folders: ${missingFiles.join(', ')}`); | ||
| return; |
There was a problem hiding this comment.
We probably should exit the program with -1 so that's friendly to integrate with pipeline that reports failures.
tools/integration/src/index.ts
Outdated
| await validatePackageJson(packageData); | ||
|
|
||
| // Validate access rules in JSON files within the `dashboards` folder | ||
| validateAccessRulesInDashboards(dashboardsPath); |
There was a problem hiding this comment.
Suggest to rename it more general, in case we have more checks later with the dashboard files.
| // Function to validate `package.json` | ||
| async function validatePackageJson(packageData: any): Promise<void> { | ||
| // Validate `name` | ||
| const namePattern = /^@instana-integration\/[a-zA-Z0-9-_]+$/; |
There was a problem hiding this comment.
This rule is only a must if the package is officially maintained by IBM. User defined package does not need to follow it. We can report it as a warning instead of error. Or we can make it configurable (error vs. warning), e.g.: when run lint against observability-as-code repo, we should enable it as error, since the repo is used to host official package.
tools/integration/src/index.ts
Outdated
| if (!namePattern.test(packageData.name)) { | ||
| throw new Error(`Invalid package name "${packageData.name}". The name must start with "@instana-integration/" followed by the package name.`); | ||
| } | ||
| console.log('Package name is valid.'); |
There was a problem hiding this comment.
These kind of logging info probably can be changed to use logger.debug.
tools/integration/src/index.ts
Outdated
| } | ||
|
|
||
| // Check for description | ||
| if (!packageData.description) { |
There was a problem hiding this comment.
It looks this can also be merged into the above code block for the required fields check.
tools/integration/src/index.ts
Outdated
| const files = fs.readdirSync(dashboardsPath); | ||
| const jsonFiles = files.filter(file => file.endsWith('.json')); | ||
|
|
||
| if(jsonFiles.length === 0){ |
There was a problem hiding this comment.
It's valid the dashboards folder is empty.
test-environment
Lint Command
| @@ -0,0 +1,618 @@ | |||
| { | |||
| "id": "QlCHQaXSTs-j5CnOKPIrEQ", | |||
There was a problem hiding this comment.
What's this dashboard used for?
tools/integration/src/index.ts
Outdated
| }) | ||
| .option('debug', { | ||
| alias: 'd', | ||
| describe: 'Enable debug mode to show detailed logs', |
There was a problem hiding this comment.
There's debug option in other commands, just to be sure we have the same description consistently.
tools/integration/src/index.ts
Outdated
| const requiredSections = [ | ||
| packageName, | ||
| 'Dashboards', | ||
| 'Go Runtime Metrics', |
There was a problem hiding this comment.
These titles are particularly for go, when it comes to other technology, title will be changed. We could think about using a set of consistent titles, e.g.: "Metrics", "Semantic Conventions".
tools/integration/src/index.ts
Outdated
| } | ||
| } catch (error) { | ||
| if ((error as AxiosError).response?.status === 404) { | ||
| if (packageData.version === '1.0.0') { |
There was a problem hiding this comment.
If it's a new package, can we just leave as is w/o restricting the init version?
tools/integration/src/index.ts
Outdated
| // Validate `name` | ||
| const namePattern = /^@instana-integration\/[a-zA-Z0-9-_]+$/; | ||
| if (!namePattern.test(packageData.name)) { | ||
| warnings.push(`Warning: Package name "${packageData.name}" does not align with the IBM package naming convention.`); |
There was a problem hiding this comment.
When enabling this w/ pipeline to observability-as-code repo, can we enforce such warnings on the pipeline. This means if there's warning detected like this, we will fail the pipeline and report back to the associated PR. This is ensure user contributed contents align w/ the rules we defined for all the official packages.
Co-authored-by: MorningSpace <[email protected]>
Co-authored-by: MorningSpace <[email protected]>
Test environment to main
No description provided.