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

chore: fix divergent files in compas package #297

Conversation

Stef3st
Copy link

@Stef3st Stef3st commented Nov 20, 2023

closes #294

Signed-off-by: Stef3st <steffen.van.den.driest@alliander.com>
@Stef3st
Copy link
Author

Stef3st commented Nov 20, 2023

The files that are still in both packages are the following:

  • src/Historing.ts
  • src/Hosting.ts
  • src/open-scd.ts
  • src/Plugging.ts
  • src/Setting.ts
  • src/translations/loader.ts (is an exact duplicate, but is needed to load the different translations)
  • src/translations/de.ts
  • src/translations/en.ts

Signed-off-by: Stef3st <steffen.van.den.driest@alliander.com>
Signed-off-by: Stef3st <steffen.van.den.driest@alliander.com>
@Stef3st Stef3st marked this pull request as ready for review November 27, 2023 08:20
Base automatically changed from 284-update-compas-oscd-build-process to main December 4, 2023 10:06
Copy link

@juancho0202 juancho0202 left a comment

Choose a reason for hiding this comment

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

It looks good, I would only like to request you to get rid of any customization related to the OCL validator, this includes of course also removing the "[WIP] Validate using OCL" entry from the packages/compas-open-scd/public/js/plugins.js file 👍

Choose a reason for hiding this comment

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

This should go and so should the Plugging mixin

Choose a reason for hiding this comment

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

Just talked to Sander and mentioned how ridiculous it is that we're modifying the Plugging mixin only to account for the OCL Validator being tagged as "WIP" and I told him if it's ok we just remove the plugin from the plugin.js list and so this file can be removed as well.

Choose a reason for hiding this comment

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

can be deleted as well since we will no longer test Plugging here

@Stef3st
Copy link
Author

Stef3st commented Dec 7, 2023

I think we need to keep the Plugging mixin for now. This is the only way for now to override the default plugin list in the distribution. This definitely needs to be fixed once OpenSCD-core is fully in place. We can make a ticket for that to keep this in mind.

Issue created: #302

@Stef3st Stef3st linked an issue Dec 7, 2023 that may be closed by this pull request
2 tasks
Copy link

@juancho0202 juancho0202 left a comment

Choose a reason for hiding this comment

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

Good to go!

CoMPAS Pull Request Overview Board automation moved this from To do to In progress Dec 7, 2023
@juancho0202 juancho0202 merged commit 3e1b4a3 into main Dec 7, 2023
6 checks passed
CoMPAS Pull Request Overview Board automation moved this from In progress to Done Dec 7, 2023
@juancho0202 juancho0202 deleted the 294-fix-divergent-files-in-compas-open-scd-vs-open-scd-package branch December 7, 2023 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Fix divergent files in compas-open-scd vs open-scd package
2 participants