Skip to content
This repository was archived by the owner on Oct 20, 2022. It is now read-only.

Stfc fix template import script#942

Open
keiranjprice101 wants to merge 12 commits intodevelopfrom
stfc_fix_template_import_script
Open

Stfc fix template import script#942
keiranjprice101 wants to merge 12 commits intodevelopfrom
stfc_fix_template_import_script

Conversation

@keiranjprice101
Copy link
Contributor

Description

Fixes the issues with the template import script and makes logging clearer.

Requires UserOfficeProject/user-office-backend#666

Motivation and Context

I previously had only properly checked the delete script and failed to properly check the importScript this now works as expected.

How Has This Been Tested

Manual Testing

Fixes

Changes

Depends on

Tests included/Docs Updated?

  • I have added tests to cover my changes.
  • All relevant doc has been updated

@keiranjprice101 keiranjprice101 requested review from a team and vyshnavi1605 and removed request for a team June 1, 2022 10:34
@keiranjprice101 keiranjprice101 added the review: please A request for an ad-hoc review. label Jun 1, 2022
Copy link
Contributor

@ACLay ACLay left a comment

Choose a reason for hiding this comment

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

Maybe it's just a lack of familiarity with the project/structure, but I can't figure out where or how this script is called or used. Could you give me a quick run through? Would that be worth adding as comments at the start of the script?

}

main();
main().catch((e) => console.error(e));
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a couple of other promise .catch methods that call console.log Should they be changed to error too?

@keiranjprice101
Copy link
Contributor Author

@ACLay The intention is for this script and the delete template script to be run periodically by jenkins to keep dev and prod templates in sync. That is why its not called anywhere else. I will add a comment to make this more clear 👍

@keiranjprice101 keiranjprice101 requested a review from ACLay June 1, 2022 13:51
@keiranjprice101 keiranjprice101 enabled auto-merge June 1, 2022 14:04
@keiranjprice101 keiranjprice101 disabled auto-merge June 1, 2022 14:04
@keiranjprice101 keiranjprice101 removed the review: please A request for an ad-hoc review. label Jul 12, 2022
@keiranjprice101 keiranjprice101 disabled auto-merge July 14, 2022 09:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants