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

refactor: type consolidation #590

Merged
merged 22 commits into from
Jun 20, 2024
Merged

refactor: type consolidation #590

merged 22 commits into from
Jun 20, 2024

Conversation

mshanemc
Copy link
Contributor

uses types for sfProject, PackageDir etc from sfdx-core
refactor some un-bundleable geLogger stuff
othr basic TS cleanup (remove assertions, simplify types)

@@ -0,0 +1,170 @@
/*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is completely independent from the e2e nut so I split it out (can run parallel)

@@ -128,6 +124,7 @@ describe('Integration tests for @salesforce/packaging library', () => {
const projectFile = JSON.parse(dxPjsonData) as ProjectJson;
expect(projectFile).to.have.property('packageDirectories').with.length(1);
expect(projectFile.packageDirectories[0]).to.include.keys(['package', 'versionName', 'versionNumber']);
assert(isPackagingDirectory(projectFile.packageDirectories[0]));
expect(projectFile.packageDirectories[0].package).to.equal(pkgName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these all presume packageDirectories[0] is the flavor that has the full packaging info. asserting that via the typeguard makes everything downstream of that assertion type-aware

@@ -649,16 +650,19 @@ export class PackageVersionCreate {
if (!packageName) throw messages.createError('errorMissingPackage', [this.options.packageId]);
}
packageObject = this.project.findPackage(
(namedPackageDir) => namedPackageDir.package === packageName || namedPackageDir.name === packageName
(namedPackageDir) =>
(isPackagingDirectory(namedPackageDir) && namedPackageDir.package === packageName) ||
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this logic matches, should it be
isPackagingDirectory(namedPackageDir) &&(namedPackageDir.package === packageName || namedPackageDir.name === packageName)?

@@ -172,3 +136,32 @@ export class Package1Version implements IPackageVersion1GP {
return (await this.connection.tooling.query<PackagingSObjects.MetadataPackageVersion>(query)).records;
}
}

const packageUploadPolling = async (
Copy link
Member

Choose a reason for hiding this comment

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

I believe this is only applicable to 1GP packages, and why it was kept under the Package1Version class. I think keeping that clarity to the consumer is good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not following. it's not exported, so there shouldn't be any consumers outside the package1Version module, right?

@WillieRuemmele WillieRuemmele merged commit 50baba7 into main Jun 20, 2024
12 of 14 checks passed
@WillieRuemmele WillieRuemmele deleted the sm/type-consolidation branch June 20, 2024 15:48
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