-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
Implement #194: remove latest
composer deps from default generated pipelines
#197
base: 2.0.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,6 @@ export const ACTION = 'laminas/laminas-continuous-integration-action@v1'; | |
export enum ComposerDependencySet { | ||
LOWEST = 'lowest', | ||
LOCKED = 'locked', | ||
LATEST = 'latest', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would not remove this. It should be possible to generate Jobs with I still see this being necessary as something has to check latest, just not every PR. |
||
} | ||
|
||
export function gatherVersions(composerJson: ComposerJson): InstallablePhpVersionType[] { | ||
|
@@ -111,10 +110,10 @@ function discoverPhpVersionsForJob(job: JobDefinitionFromFile, config: Config): | |
|
||
function discoverComposerDependencySetsForJob(job: JobDefinitionFromFile, config: Config): ComposerDependencySet[] { | ||
const dependencySetFromConfig = job.dependencies | ||
?? (config.lockedDependenciesExists ? ComposerDependencySet.LOCKED : ComposerDependencySet.LATEST); | ||
?? (config.lockedDependenciesExists ? ComposerDependencySet.LOCKED : ComposerDependencySet.LOWEST); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure if this makes sense at all. if there is no lock-file, latest would be installed in projects as well. since we are not affected in the laminas components (as we do have lockfiles), I'd say there is no need to touch this. |
||
|
||
if (isAnyComposerDependencySet(dependencySetFromConfig)) { | ||
return [ ComposerDependencySet.LOWEST, ComposerDependencySet.LATEST ]; | ||
return [ ComposerDependencySet.LOWEST ]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should keep this as it is explicitly required from a manually written config entry. If some component does not need/want |
||
} | ||
|
||
return [ dependencySetFromConfig ]; | ||
|
@@ -295,7 +294,7 @@ function createJobsForTool( | |
if (tool.executionType === ToolExecutionType.STATIC) { | ||
const lockedOrLatestDependencySet: ComposerDependencySet = config.lockedDependenciesExists | ||
? ComposerDependencySet.LOCKED | ||
: ComposerDependencySet.LATEST; | ||
: ComposerDependencySet.LOWEST; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above. If there is no lockfile, having lowest would be super weird imho. |
||
|
||
return [ | ||
createJob( | ||
|
@@ -343,18 +342,7 @@ function createJobsForTool( | |
config.ignorePhpPlatformRequirements[version] ?? false, | ||
config.additionalComposerArguments, | ||
beforeScript, | ||
), tool) as JobFromTool, | ||
|
||
createJob(tool.name, createJobDefinition( | ||
tool.command, | ||
version, | ||
ComposerDependencySet.LATEST, | ||
config.phpExtensions, | ||
config.phpIni, | ||
config.ignorePhpPlatformRequirements[version] ?? false, | ||
config.additionalComposerArguments, | ||
beforeScript, | ||
), tool) as JobFromTool, | ||
), tool) as JobFromTool | ||
)); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,9 +6,6 @@ | |
}, | ||
{ | ||
"name": "PHPUnit [8.2, locked]" | ||
}, | ||
{ | ||
"name": "PHPUnit [8.2, latest]" | ||
} | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what renovate/dependabot config entry would be required to have these updates?
besides this, it seems that lockfile updates do not properly handle latest. So we might want to have an additional action to be registered - or we go the way that we do only run latest for auto-contributions such as dependabot/renovate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lockfile maintenance is as if you run composer update on the PHP version specified in
config.platform.php
, so we won't get updates that require a higher minimum PHP version.