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

add: verification of item referencing for validated_by relationship #18

Closed
wants to merge 5 commits into from
Closed

Conversation

fkromer
Copy link
Contributor

@fkromer fkromer commented Jun 5, 2016

I added some verification about items which reference other items with a validated_by relationship but which are not defined in the *.py or *.rst (or other files supported by sphinx). I added the reference to item SW_TEST_010 (which is not defined) in addition to item SW_TEST_005 (which is also not defined) in item SW_REQ_005 of the autodoc test. This makes obvious that the functionality does also work if more than one missing item is referenced with the same relationship. Basically the functionality does work.

The reason why this is a "support" request and no actual pull request:
As this feature is added in class ItemDirective which is called every time an item directive has been found the verification is executed several times before all items and item relationships are known. I assume the equivalent functionality should be implemented in function process_item_nodes() instead. But I am not sure how to implement it in process_item_nodes() (or somewhere else) and need some hint...

This draft issue can be observed on my fork when the sphinx-build is run over autodoc test (e.g. sphinx-build -b html . _build): The console output about missing referenced items is correct when in the autodoc test only prod_code.py has been changed. If test_code.py has been changed in addition to or without prod_code.py the console output shows some items which do actually have existing items (assumed root cause ~ functionality implemented in ItemDirective.run() instead of proess_item_nodes()).

Basically the logic could be used to check item referencing with all other relationship types as well. (The dict holding the "validated_by" referenced item id list as value would need a item id list for every relationship in this pull request implementation variant.)

fkromer added 3 commits June 5, 2016 12:10
- correct output (list of missing referenced items for every
referencing item) when prod_code.py has changed
- wrong output when test_code.py (with or without change of
prod_code.py) has changed -> the logic seems to be relocated
into another function (when all items and relationships are known)
add:
- missing validated_by item SW_TEST_010 for item SW_REQ_005
in autodoc test
change:
- dict of referencing items as keys, list of referenced 'validated_by'
items as value
fkromer added 2 commits July 7, 2016 10:03
into env-update method
(verification is executed once after all doctrees are up-to-date
and not several times after single doctrees are updated ~ 
files which have changed)
@fkromer
Copy link
Contributor Author

fkromer commented Jul 7, 2016

I refactored the item ID referencing verification into item_verification(..) which is invoked by sphinx-doc as env-updated event once when all item IDs are known.

The autodoc test terminal output looks like this (independent of whether or not files have changed):

Running Sphinx v1.3.4
loading pickled environment... done
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 0 source files that are out of date
updating environment: 0 added, 0 changed, 0 removed
WARNING: missing item ID(s) ['SW_TEST_005', 'SW_TEST_010'] (referenced by SW_REQ_005 as validated_by)
WARNING: missing item ID(s) ['SW_TEST_001'] (referenced by SW_REQ_001 as validated_by)
looking for now-outdated files... none found
no targets are out of date.
build succeeded, 2 warnings.

As the mentioned item IDs do not exist for the related referencing item IDs the functionality seems to work as expected now.

(The logic could be used to check item referencing with all other relationship types as well...)

@fkromer
Copy link
Contributor Author

fkromer commented Jul 15, 2016

@ociu Can we merge this feature into the next version or shall we add the verification of the other relationship types as well? When will you release the next version?

@fkromer fkromer changed the title add: draft for verification of item referencing for validated_by relationship add: verification of item referencing for validated_by relationship Jul 15, 2016
@fkromer
Copy link
Contributor Author

fkromer commented Oct 8, 2016

@SteinHeselmans @ociu Could you guys review my changes? If there is no issue it would be great to have the functionality merged...

@SteinHeselmans
Copy link

@fkromer , I had a look at your changes here. And i think my #21 implements something similar but in a more general way. My pull request implements an automatic reverse relationship (e.g. if you define a validates relationship from item_a to item_b, it will automatically create the reverse relationship using validated_by from item_b to item_a), and displays every relationship when rendering items as hyperlinks.

for this, my pull request creates 'placeholder' items when creating an automatic reverse link on an item which is not created yet. When then the 'placeholder' item is defined (in another file, or further down that same file), it will overwrite the 'placeholder' keeping the already available relationships. In the end no 'placeholder' items can remain, otherwise it throws an error. More details in my pull request...

We have been using that version in #21 for a few months now in my company (Melexis) with good success...

Could you review (and/or try) if my solution fits your needs? If so, we can maybe drop this one, and use the completer 21?

@fkromer
Copy link
Contributor Author

fkromer commented Nov 22, 2016

@SteinHeselmans I do agree with you. The more general the solution is the better. I do not trust my review capabilities as much as tests :) Have you figured out to get the test of #21 run locally?

@SteinHeselmans
Copy link

i don't know how to run/alter the test cases for this plugin, no experience in that. Only thing i can rely on is our project documentation at Melexis, which improved a lot with those changes.

Let's park the discussion here, and continue in #21. When we conclude there, we can still look back to this pull request to see if it adds value...

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.

None yet

2 participants