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

Pulling Base CSV Checking Feature #8

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

sfrinaldi
Copy link
Collaborator

@sfrinaldi sfrinaldi commented Sep 13, 2024

Tested on develop branch to verify the added attribute ties done in this branch for the RVM don't break on ones that do not have this information / csv headers yet. Appears to work still without breaking.

Feature includes doorstop base check during the doorstop import command for csv files. Workflow will still continue but a warning_log.txt file will generate (as well as console warning messages during the run) with just the duplicate UID errors it found when parsing through each csv file.

Also skips over / ignores comment lines now for the invalid UID warnings that would generate before.

Adding RVM attributes for being able to pull those values easier from doorstop (in the correct manner) for implementing with the modified rtv_doorstop fork. Removed testing files so repo doesn't need to be removed after cloning (or to be implemented as a submodule or something else). (Testing files of requirements cause there to be multiple 'roots' detected as there is a built tree already)

Example warning_log.txt:

image

More features will be added to this whether in a separate repo (requirements-tools) or using available tools (like csv-lint). Pros for merging this initial feature earlier is current requirements being developed / reviewed in Gitlab will be able to get this warning_log.txt (locally / gitlab workflow will need to be adjusted for adding this file as an artifact) and duplicate uid messages in the workflow logs.

Testing out having RVM built into doorstop / seeing how it would be for automation. Not completed yet / first commits. This is non-functional at the moment.
Commenting out RVM
Adjusting new RTM values.
Will allow repo to be in pearl_requirements / shorten time (could be a submodule).
Trying to just have logging write to a file for ones that are warnings.
@sfrinaldi sfrinaldi self-assigned this Sep 13, 2024
@sfrinaldi sfrinaldi added the enhancement New feature or request label Sep 13, 2024
@sfrinaldi
Copy link
Collaborator Author

@patrickingraham Let me know if you think this is worth pulling to main in its initial state so that the current branches (especially when compiling locally) can see this information for when requirements are being adjusted. Otherwise I can just wait on this PR / continue to work on it.

@douglase
Copy link
Collaborator

@sfrinaldi have you tested against https://github.com/douglase/doorstop_requirements_template/ or do we need to update that too?

@sfrinaldi
Copy link
Collaborator Author

sfrinaldi commented Nov 1, 2024

Will double check! Might need to be updated.

@sfrinaldi
Copy link
Collaborator Author

sfrinaldi commented Nov 1, 2024

@douglase Alright, so I checked it out and for that repo it looks like its what is used in requirements_tools but also a lot of the functionality for the newer doorstop (as it can publish to latex / markdown / excel actually now and not just html) doesn't require pandocs. I see there is a UASAL fork uasal/doorstop_requirements_template for that repo you linked (douglase/doorstop-requirements-template) which looks like it is out of date in comparison.

The graphviz is still being generated and added (in pretty much the same fashion but we have it stop at a certain level (4) as then it becomes a bit unreadable). Did we want to have it integrated / use travisci? Let me know if there was something else you were referring too that I didn't cover.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants