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.

Sign up for GitHub

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

Jump to bottom

chore: fix divergent files in compas package #297

Merged
merged 4 commits into from
Dec 7, 2023
Merged

chore: fix divergent files in compas package #297

merged 4 commits into from
Dec 7, 2023

Conversation

Stef3st
Copy link

@Stef3st Stef3st commented Nov 20, 2023

closes #294

@Stef3st Stef3st self-assigned this Nov 20, 2023
@Stef3st
Copy link
Author

Stef3st commented Nov 20, 2023 •
edited
Loading

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

@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
juancho0202
juancho0202 reviewed Dec 5, 2023
View reviewed changes
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 👍

packages/compas-open-scd/test/unit/CompasPlugging.test.ts
Copy link

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

packages/compas-open-scd/src/Plugging.ts
Copy link

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.

packages/compas-open-scd/test/unit/mock-compas-plugger.ts
Copy link

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 •
edited
Loading

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
Fix divergent files in compas-open-scd vs open-scd package #294
Closed
2 tasks
juancho0202
juancho0202 approved these changes Dec 7, 2023
View reviewed changes
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!

@juancho0202 juancho0202 merged commit 3e1b4a3 into main Dec 7, 2023
6 checks passed
@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
Reviewers

@juancho0202 juancho0202 juancho0202 approved these changes

@pascalwilbrink pascalwilbrink Awaiting requested review from pascalwilbrink

Assignees

@Stef3st Stef3st

Labels
None yet
Projects
CoMPAS Pull Request Overview Board
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

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