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/65 support fault event model with fha and sns properties #68

Merged

Conversation

kostobog
Copy link
Collaborator

@kostobog kostobog commented Mar 12, 2024

Resolves #65

@kostobog kostobog requested a review from blcham March 12, 2024 08:05
Copy link
Contributor

@blcham blcham left a comment

Choose a reason for hiding this comment

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

We need to talk about refactoring all entities, how hard it would be. Let's define conventions here, add new entities with the convention in mind, and leave refactoring for a separate issue.

src/main/java/cz/cvut/kbss/analysis/model/Item.java Outdated Show resolved Hide resolved
### http://onto.fel.cvut.cz/ontologies/fta-fmea-application/from
fta-fmea:from rdf:type owl:DatatypeProperty ;
rdfs:domain fta-fmea:failure-rate-requirement .


### http://onto.fel.cvut.cz/ontologies/fta-fmea-application/has-ata-code
fta-fmea:has-ata-code rdf:type owl:DatatypeProperty .
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to discuss naming conventions and how hard it would be to refactor everything with those conventions. It won't be most-likely easier than now.

Conventions from CSAT:

  • entities contain only small letters, with words separated by "-" (even classes)
  • object properties should start with :has-
  • datatype properties typically do not start with :has-

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do not see a problem of refactoring. The only problem I see is that existing data won't be compatible and would need to be refactored as well.

However, I suggest to do this in a separate ticket.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suggest to do this in a separate ticket.
The only problem I can see is that we will have to also refactor old data, for example data in fta-fmea-demo.

I have some reservation for the proposed naming conventions:

  • object properties should start with 'has' - in my opinion some properties do not need it. For example, :activatedBy should be refactored as :has-activated-by. I would prefer to refactor it as :activated-by.

Copy link
Contributor

Choose a reason for hiding this comment

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

activates           
is-activated-by         
has-activating-behaviour

Copy link
Contributor

Choose a reason for hiding this comment

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

Conventions will be solved by issue kbss-cvut/fta-fmea-ui#198

@kostobog kostobog force-pushed the feature/65-support-fault-event-model-with-fha-and-sns-properties branch from 3798901 to e6df4d0 Compare March 13, 2024 08:44
@blcham
Copy link
Contributor

blcham commented Mar 13, 2024

Let's agree on how it should be refactored in kbss-cvut/fta-fmea-ui#198

@kostobog kostobog force-pushed the feature/65-support-fault-event-model-with-fha-and-sns-properties branch from e6df4d0 to c0f7483 Compare March 14, 2024 12:08
@kostobog kostobog force-pushed the feature/65-support-fault-event-model-with-fha-and-sns-properties branch from c0f7483 to 82573b8 Compare March 14, 2024 13:05
@kostobog kostobog merged commit 8ff3a53 into main Mar 14, 2024
1 check passed
@blcham blcham deleted the feature/65-support-fault-event-model-with-fha-and-sns-properties branch May 18, 2024 08:39
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.

Extend Fault Event type model to contain additional attributes (quantity, ..)
2 participants