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

Set default metrics for all datasets #229

Merged
merged 10 commits into from
Nov 22, 2023

Conversation

SwordSaintLancelot
Copy link

Why are you creating this Pull Request?

The default metric has been added to all the layers of every dataset for the US GHG center.

@SwordSaintLancelot SwordSaintLancelot self-assigned this Nov 20, 2023
Copy link

netlify bot commented Nov 20, 2023

Deploy Preview for ghg-demo ready!

Name Link
🔨 Latest commit a76f020
🔍 Latest deploy log https://app.netlify.com/sites/ghg-demo/deploys/655d21d9fad85d0009d5206e
😎 Deploy Preview https://deploy-preview-229--ghg-demo.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@j08lue j08lue left a comment

Choose a reason for hiding this comment

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

@SwordSaintLancelot there seem to be indentation errors that caused the preview build to fail. See the CI job logs for details, please.

@SwordSaintLancelot
Copy link
Author

SwordSaintLancelot commented Nov 20, 2023

@j08lue I did look into the logs and it mentioned that the issue is with duplicate mapping keys for all the dataset files. I checked the indentation and it seems to be correct (at least similar to the example that you did)
I removed the analysis key from a the datasets that showed the error.

Copy link
Collaborator

@slesaad slesaad left a comment

Choose a reason for hiding this comment

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

Also don't push .veda/ui changes?

@slesaad
Copy link
Collaborator

slesaad commented Nov 21, 2023

@SwordSaintLancelot don't add exclude to all the datasets, only to those that already had it

@SwordSaintLancelot
Copy link
Author

Also don't push .veda/ui changes?

Reverted them to their original state.

package.json Outdated
@@ -1,7 +1,7 @@
{
"name": "veda-config",
"description": "Configuration for Veda",
"version": "1.0.0-beta.2",
"version": "0.1.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this branch here should be rebased to current develop or something? Where you did you branch off from, @SwordSaintLancelot?

Copy link
Author

@SwordSaintLancelot SwordSaintLancelot Nov 21, 2023

Choose a reason for hiding this comment

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

I did branch it from develop. I will rebase it and push again. Also, this package was updated when I tried to view the changes in local netlify using the command yarn serve.

Copy link
Collaborator

@j08lue j08lue Nov 21, 2023

Choose a reason for hiding this comment

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

Thanks. I was just worried about the "reverting" commits above. If these changes are already in develop by now and meant to be there, your commits here would undo them.

Copy link
Author

Choose a reason for hiding this comment

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

I think there were a few changes that happened when I ran the yarn serve and that's what modified the /.veda/ui folder along with package.json but I changed those to original package.json which was in the develop branch.

@SwordSaintLancelot SwordSaintLancelot marked this pull request as draft November 22, 2023 14:58
@SwordSaintLancelot SwordSaintLancelot marked this pull request as ready for review November 22, 2023 15:00
@SwordSaintLancelot SwordSaintLancelot requested review from j08lue and removed request for j08lue November 22, 2023 15:51
Copy link
Collaborator

@j08lue j08lue left a comment

Choose a reason for hiding this comment

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

Looks good, great work getting this ready!

@SwordSaintLancelot SwordSaintLancelot merged commit 75b581b into develop Nov 22, 2023
5 checks passed
@SwordSaintLancelot SwordSaintLancelot deleted the update-default-metrics-for-dataset-layers branch November 22, 2023 16:47
@j08lue j08lue changed the title updated the default metrics for the datasets Set default metrics for all datasets Nov 23, 2023
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.

3 participants