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

Feature/api/mrxn23 97 delete cost surface [MRXN23-97] #1529

Merged
merged 5 commits into from
Oct 6, 2023

Conversation

KevSanchez
Copy link
Collaborator

Substitute this line for a meaningful title for your changes

Overview

Please write a description. If the PR is hard to understand, provide a quick explanation of the code.

Designs

Link to the related design prototypes (if applicable)

Testing instructions

Please explain how to test the PR: ID of a dataset, steps to reach the feature, etc.

Feature relevant tickets

Link to the related task manager tickets


Checklist before submitting

  • Meaningful commits and code rebased on develop.
  • If this PR adds feature that should be tested for regressions when
    deploying to staging/production, please add brief testing instructions
    to the deploy checklist (docs/deployment-checklist.md)
  • Update CHANGELOG file

@vercel
Copy link

vercel bot commented Oct 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marxan ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 6, 2023 10:29am

@KevSanchez
Copy link
Collaborator Author

Part of the GET endpoints functionality is in this PR, as it was shared on the CostSurfaceService. Thought it was not worth the hassle to rearrange everything, when the GETs PR is going to be merged on top of this one too.

@KevSanchez KevSanchez changed the base branch from develop to chore/cost-surfaces-data-migration October 2, 2023 09:05
@KevSanchez KevSanchez changed the base branch from chore/cost-surfaces-data-migration to feature/api/cost-surface-migrations October 2, 2023 09:05
Base automatically changed from feature/api/cost-surface-migrations to develop October 3, 2023 11:20
@KevSanchez KevSanchez force-pushed the feature/api/MRXN23-97-delete-cost-surface branch from 3d65805 to 2823402 Compare October 3, 2023 16:26
Comment on lines +62 to +64
if (piece === ClonePiece.ScenarioInputFolder) {
this.logger.log('SCENARIO INPUT FOLDER');
}
Copy link
Member

Choose a reason for hiding this comment

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

just a question: I'm almost always in favour (actually fanatical) about meaningful logging that could help when inspecting live systems, but I'm not sure about this - why do we add info logging for this specifically? and what is this log line meant to convey?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, sorry about this one. This was part of the process of deciphering the whole cloning mystery. Seems like it slipped away.

@KevSanchez KevSanchez force-pushed the feature/api/MRXN23-97-delete-cost-surface branch from 7c02932 to e1c2fac Compare October 4, 2023 14:40
@KevSanchez KevSanchez merged commit 5cc437a into develop Oct 6, 2023
52 checks passed
@KevSanchez KevSanchez deleted the feature/api/MRXN23-97-delete-cost-surface branch October 6, 2023 11:15
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