Skip to content

Conversation

@tolisso
Copy link
Collaborator

@tolisso tolisso commented Jun 21, 2023

Added new project creation with functional blocks of a IEC 61131 project

@tolisso tolisso requested a review from qradimir June 21, 2023 16:35
@tolisso tolisso self-assigned this Jun 21, 2023
Copy link
Collaborator

@qradimir qradimir 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 to me. I would like to see some tests for new functionallity

.idea/aws.xml Outdated
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a redundant config file, consider to drop it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

.idea/misc.xml Outdated
<component name="ProjectRootManager" version="2" languageLevel="JDK_11" default="true" project-jdk-name="11" project-jdk-type="JavaSDK">
<output url="file://$PROJECT_DIR$/out" />
</component>
<component name="ProjectType">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where it comes from?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed

import kotlin.io.path.Path
import kotlin.io.path.readText

class Iec61131Xml {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there is no major benefit of making all serializable classes nested inside a single class. Having them all top-level is fine.

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