handle default prefixes #1304
handle default prefixes #1304
Conversation
Jamie-SA
requested review from
dhurlburtusa,
rjyounes,
sa-bpelakh and
stevenchalem
stevenchalem
left a comment
stevenchalem
left a comment
There was a problem hiding this comment.
I have tested this on Windows and it all looks good. I like the changes you made.
stevenchalem
left a comment
stevenchalem
left a comment
There was a problem hiding this comment.
This needs a release note.
Co-authored-by: Rebecca Younes <[email protected]>
This reverts commit 2797780.
|
I think I have got this back to what it was originally with Rebecca's release note update. |
…nticarts/gist into 898-handle-default-prefixes
stevenchalem
left a comment
stevenchalem
left a comment
There was a problem hiding this comment.
Tested successfully under Windows 11.
|
@rjyounes why is it again that we set "git config core.filemode false" on this repo? It seems like the wrong option, I think the repo should track executable flags (even if Windows doesn't care). I keep having to set setup.cmd to executable every time I change branches (or every time I change branches that change that file). |
|
@Jamie-SA Regarding the question about |
| @@ -0,0 +1,4 @@ | |||
| ### Patch Updates | |||
|
|
|||
| - Added a test in the git pre-commit hook to check for default PREFIX declarations in ontology files and cause the commit to fail if it finds one. For this to work, the setup.cmd command needs to be re-run to update the git hook. Issue [#898](https://github.com/semanticarts/gist/issues/898). | |||
There was a problem hiding this comment.
Why don't we have the pre-commit hook remove the prefix declaration, in the same way that the serializer reformats and makes some modifications?
There was a problem hiding this comment.
One more issue: the pre-commit hook issues an error and blocks the commit when I have a file either in the ontologies directory or the root directory that is not being committed. It should be looking only at the files being committed, like the serializer.
This is another reason not to include the base directory (comment below), but that will not fully address it, since such a file could exist in my ontologies directory.
There was a problem hiding this comment.
Mainly because it was faster/easier to just block the commit and I was trying to get this done so the release could get done.
There was a problem hiding this comment.
Understood. That's why I want to postpone this to the next release when we can take the time to do it right. There's no need for it to go into 14.0.0.
|
|
||
| # Get root directory of this git repository | ||
| base_dir=$(git rev-parse --show-toplevel) | ||
| # For repositories that have their ontology files in the project root directory |
There was a problem hiding this comment.
I understand that you want to make this general, but since in this repository the ontologies are stored in the ontologies subdirectory, I don't think we should include this. Furthermore, other configurations are possible - e.g., for one client, we had models/ontologies. The point being it's not going to be all-purpose, and users are possibly going to have to modify the path if they copy this to another repo.
| @@ -0,0 +1,18 @@ | |||
| #!/usr/bin/env sh | |||
|
|
|||
| # This file should be copied into the .git/hooks directory of the project. | |||
There was a problem hiding this comment.
| # This file should be copied into the .git/hooks directory of the project. | |
| # This file should be copied into the .git/hooks directory of the project. This is done automatically by running setup.cmd. |
| print("ERROR: there is a default PREFIX statement in file: ", filePath) | ||
| print("It must be removed from the file before you can commit your other changes.") | ||
| changed = True | ||
| # The following line would fix the file, then you would need to "git add filename" |
There was a problem hiding this comment.
The serializer modifies files without having to re-add them.
|
Moved to next release. We need to get the right behavior before merging to develop. |
|
We should also break these two functions up into two different PRs:
|
|
I had them as two separate PRs, you merged them. |
|
Yes, that was a mistake. But one targeted the other so I thought you intended to merge them. |
Since they modify the same files, it made since to me to stack them. Submitting them separately without one stacked on the other may result in merge conflicts. Is there a preference in which one to focus on first? |
|
Ensuring the user has the right pre-commit hook is more important than removing the default prefix. |
Closes #898
This PR prevents commits if an ontology file has a default prefix declared.
Everyone will need to run setup.cmd again.