Skip to content
This repository has been archived by the owner on Mar 18, 2024. It is now read-only.

Refactor PackageManifest class #743

Merged
merged 4 commits into from
Oct 29, 2021
Merged

Conversation

aly76
Copy link
Contributor

@aly76 aly76 commented Oct 28, 2021

Change method of instantiation of PackageManifest to factory method. This allows
the package.xml to be parsed when PackageManifest is instantiated, as
constructors cannot be asynchronous. By enforcing the xml to be parsed at the time of object
creation, the instance methods are guaranteed to have access to the manifest.

Fix bug where 'types' is undefined and causes an error trying to reference a
property on type undefined.

Fixes https://github.com/dxatscale/sfp-cli/issues/47

Change method of instantiation of PackageManifest to factory method. This allows
the package.xml to be parsed when PackageManifest is instantiated, as
constructors cannot be asynchronous. By enforcing the xml to be parsed at object
creation, the instance methods are guaranteed to have access to the manifest.

Fix bug where 'types' is undefined and causes an error trying to reference a
property on type undefined.
@aly76 aly76 added the bug Something isn't working label Oct 28, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 28, 2021

Codecov Report

Merging #743 (a61331f) into develop (1aef0c8) will increase coverage by 0.21%.
The diff coverage is 68.29%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #743      +/-   ##
===========================================
+ Coverage    64.59%   64.81%   +0.21%     
===========================================
  Files           44       44              
  Lines         1466     1475       +9     
  Branches       264      268       +4     
===========================================
+ Hits           947      956       +9     
  Misses         519      519              
Impacted Files Coverage Δ
packages/core/src/package/PackageManifest.ts 74.50% <66.66%> (+5.46%) ⬆️
packages/core/src/package/SFPPackage.ts 83.33% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b7634ae...a61331f. Read the comment docs.

@aly76 aly76 added refactor and removed bug Something isn't working labels Oct 28, 2021
Copy link
Contributor

@azlam-abdulsalam azlam-abdulsalam left a comment

Choose a reason for hiding this comment

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

Nice!

let packageManifest:PackageManifest = new PackageManifest(sfpPackage.mdapiDir);
sfpPackage._payload = await packageManifest.getManifest();
const packageManifest:PackageManifest = await PackageManifest.create(sfpPackage.mdapiDir);
sfpPackage._payload = packageManifest.manifest;
sfpPackage._triggers = packageManifest.fetchTriggers();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a const here, cant we just short circuit it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ignore me :)

@azlam-abdulsalam azlam-abdulsalam merged commit f5cb28b into develop Oct 29, 2021
@azlam-abdulsalam azlam-abdulsalam deleted the refactor/package-manifest branch October 29, 2021 03:14
@azlam-abdulsalam azlam-abdulsalam added this to the Release - November 21 milestone Nov 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants