Namnn/need per id json #960

nhatnamnguyengtvthcm · 2023-08-04T09:31:20Z

In the builder, I build one builder which is name build_needs_id_json to build Json file per need id
Using command: make docs-html-fast to run.
Finally, in the "_build" folder will be gen the "needs_id" folder include all file json of need id.

danwos

Thanks for the new builder.
It would be great also to get a test-case for this builder.

danwos · 2023-08-07T06:48:42Z

docs/conf.py

@@ -379,13 +379,14 @@ def custom_defined_func():

 # build needs.json to make permalinks work
 needs_build_json = True
-
+# build needs per id
+needs_per_id = True


Why does the Sphinx-Needs doc configuration need this?

I change commend: "build single need.json to make detail pannel". Is this ok?

Will the detail pannel be part of Sphinx-Needs.
AFAIK it is not yet available, so we can activate this option when really needed.

Right now I don't see a use-case why this needs to be activated.

Will I by pass this and needs-id folder always generated when build docs?

docs/configuration.rst

sphinx_needs/builder.py

nhatnamnguyengtvthcm · 2023-08-07T11:49:31Z

Thanks for the new builder. It would be great also to get a test-case for this builder.

pytest tests/test_needs_id_builder.py => for run my test

danwos

Thanks for the changes and the test cases

Found only 2-3 minor things.

danwos · 2023-08-07T11:57:33Z

docs/configuration.rst

-Exports forder ``needs_per_id_build_path`` include all file ``needs.json`` for every needs-id.
+.. hint::
+
+   The created ``needs_id`` folder gets stored in the ``outdir`` of the current builder. The final location is e.g. _build/needs_id


e.g. _build/needs_id -> Is needs_id a builder here?

e.g. _build/needs_id -> Is needs_id a builder here?

This variable configs the folder name which is area generate all single file json for every needs-id

danwos · 2023-08-07T11:58:55Z

docs/configuration.rst

+
+needs_build_json_per_id 
+~~~~~~~~~~~~~~~~~~~~~~~
+Builds a single ``needs.json`` file for every needs-id.


Will there be only one needs,json file?
Do all the exports get the same file name?

docs/configuration.rst

danwos · 2023-08-07T12:43:24Z

Ohh and we need an entry for:

changelog.rst, to document the change for the version
AUTHORS file, if you like.

Please use also versionadded directive in the conf-docs, so that a user knows when the new config-option/builder was added.

We also need an update for docs/build.rst, which is totally missing currently

nhatnamnguyengtvthcm · 2023-08-08T03:26:57Z

Ohh and we need an entry for:

changelog.rst, to document the change for the version

AUTHORS file, if you like.

Please use also versionadded directive in the conf-docs, so that a user knows when the new config-option/builder was added.

We also need an update for docs/build.rst, which is totally missing currently

which rule setting version?I wonder how define it

danwos

Thanks for the update.
Some minor findings, which are all documentation related.

docs/builders.rst

docs/conf.py

danwos · 2023-08-09T08:01:02Z

docs/configuration.rst

-This option works like needs_build_json. 
+.. _needs_build_json_per_id:
+
+needs_build_json_per_id 


Please document first needs_build_json_per_id then needs_build_json_per_id_path, as needs_build_json_per_id_path is based on needs_build_json_per_id

danwos · 2023-08-24T07:57:28Z

The logic here is not reusing the Needsfile class, which contains some important handling and configuration.
We should update/extend Needsfile to provide a function to store the data in the needed format.
But data preparation shall be reused from it.

https://github.com/useblocks/sphinx-needs/blob/master/sphinx_needs/needsfile.py

I guess it would be enough to provide a different write_json() function with your already existing logic.
Maybe write_jsons?

sphinx-needs/sphinx_needs/needsfile.py

Lines 96 to 104 in 5b983a4

    
           def write_json(self, needs_file: str = "needs.json") -> None: 
        
               # We need to rewrite some data, because this kind of data gets overwritten during needs.json import. 
        
               self.needs_list["created"] = datetime.now().isoformat() 
        
               self.needs_list["current_version"] = self.current_version 
        
               self.needs_list["project"] = self.project 
        
               needs_json = json.dumps(self.needs_list, indent=4, sort_keys=True) 
        
               file = os.path.join(self.outdir, needs_file)

danwos

Thanks for the changes, looks a little cleaner.
But I think 2 things we should change...

danwos · 2023-08-25T11:39:28Z

sphinx_needs/builder.py

+            needs_list = NeedsList(config, self.outdir, self.srcdir)
+            needs_list.wipe_version(version)
+            needs_list.add_need(version, need)
+            for need_filter in filters.values():


I think adding the filters make no sense is in this case, as they would get repeated in each json file.
Just ignore them.

danwos · 2023-08-25T11:42:25Z

sphinx_needs/needsfile.py

@@ -131,6 +131,21 @@ def load_json(self, file) -> None:

            self.log.debug(f"needs.json file loaded: {file}")

+    def write_jsons(self, needs_file, needs_path) -> None:


If you have the loop for the needs outside the write_jsons function, why can't you just use the existing write-json?

danwos

Looks good to me. Thanks for the changes 👍

chrisjsewell

See comments

chrisjsewell · 2023-08-28T06:44:49Z

Heya @nhatnamnguyengtvthcm as you can see I'm making quite a few changes at the moment 😅 but I'll circle back round and have a look at these builder PRs soon cheers

nhatnamnguyengtvthcm · 2023-08-28T06:46:50Z

Heya @nhatnamnguyengtvthcm as you can see I'm making quite a few changes at the moment 😅 but I'll circle back round and have a look at these builder PRs soon cheers

yeah. I see your comment and try to follow you

nhatnamnguyengtvthcm · 2023-08-28T06:49:33Z

about test cases.I removed all new folders and used the doc_needs_builder because the new builder has the same environment as needs needs-builder

nhatnamnguyengtvthcm · 2023-08-28T11:30:01Z

Heya @nhatnamnguyengtvthcm as you can see I'm making quite a few changes at the moment 😅 but I'll circle back round and have a look at these builder PRs soon cheers

I see make test fail some case which was not changed from my side. Can you assist me in checking it @chrisjsewell ?

danwos

Some minor findings.
Maybe recheck also the provided docs. They are sometimes hard to understand.

docs/conf.py

docs/changelog.rst

docs/configuration.rst

sphinx_needs/builder.py

danwos

One last small finding :)
Rest looks 👍

docs/configuration.rst

r-o-b-e-r-t-o · 2023-09-14T09:35:48Z

LGTM. Contribution approved.

nhatnamnguyengtvthcm and others added 14 commits July 7, 2023 17:35

gen needs.json per needs.id and command some service git to bypass test

eb546d4

update review 11/07

600ad3f

update review 11/07

0228b24

update conf

9a698b0

config variable for needs_id forder path

3de0647

update env config for needs per id

dbb52c0

add configuration for needs per id~

01320d4

add configuration for needs per id~

177d3e3

fix-ci-lint-err

efbe1fa

fix-ci-lint-err

04f5019

fix-ci-lint-err

be61481

fix-ci-lint-err

103cc2f

fix-ci-lint-err

274842f

fix-ci-lint-err

1c2af5b

danwos self-requested a review August 4, 2023 13:39

danwos requested changes Aug 7, 2023

View reviewed changes

nhatnamnguyengtvthcm added 2 commits August 7, 2023 18:40

add test-case and fix some setting for need per id builder

9136af8

add test-case and fix some setting for need per id builder

26a18b8

nhatnamnguyengtvthcm added 2 commits August 7, 2023 18:52

add test-case and fix some setting for need per id builder

ac7c5fe

add test-case and fix some setting for need per id builder

618e1b0

danwos requested changes Aug 7, 2023

View reviewed changes

add test-case and fix some setting for need per id builder

8802a04

nhatnamnguyengtvthcm added 4 commits August 9, 2023 14:17

add test-case and fix some setting for need per id builder

55e0cd3

add test-case and fix some setting for need per id builder

6462295

add test-case and fix some setting for need per id builder

dbb9f3f

add test-case and fix some setting for need per id builder

13c1ca7

danwos requested changes Aug 9, 2023

View reviewed changes

nhatnamnguyengtvthcm added 3 commits August 21, 2023 19:34

merge version 1.3.0

c2ef673

fix merge version 1.3.0

5e8fc85

fix merge version 1.3.0

71988c4

move kogic in needs id builder into needsfile

a0c771b

danwos requested changes Aug 25, 2023

View reviewed changes

nhatnamnguyengtvthcm added 2 commits August 25, 2023 19:15

move kogic in needs id builder into needsfile

4d06008

move kogic in needs id builder into needsfile

a5af64f

danwos approved these changes Aug 25, 2023

View reviewed changes

chrisjsewell requested changes Aug 25, 2023

View reviewed changes

using doc_needs_builder for test needs_id builder

9077dca

nhatnamnguyengtvthcm added 4 commits August 28, 2023 15:08

mofify configuration.rst

025cfc5

merge version 1.3.0

dafb701

change version needs_id builder

cf47fd4

fix test case for needs_id_builder

b84e4cc

nhatnamnguyengtvthcm added 4 commits August 30, 2023 14:11

update version 1.3.0

ef926b1

update version 1.3.0

0ac8a56

update version 1.3.0

3a10072

update version 1.3.0

49e91cb

danwos requested changes Aug 31, 2023

View reviewed changes

docs/conf.py Outdated Show resolved Hide resolved

docs/changelog.rst Outdated Show resolved Hide resolved

docs/configuration.rst Outdated Show resolved Hide resolved

docs/configuration.rst Outdated Show resolved Hide resolved

sphinx_needs/builder.py Outdated Show resolved Hide resolved

refactor confiuration file and builder

43689e5

danwos requested changes Sep 5, 2023

View reviewed changes

docs/configuration.rst Show resolved Hide resolved

add version 1.4.0 in needs_build_json_per_id_path configurations.rst

ba544e7

danwos approved these changes Sep 6, 2023

View reviewed changes

danwos merged commit 1fc6bde into useblocks:master Sep 6, 2023
11 of 12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Namnn/need per id json #960

Namnn/need per id json #960

nhatnamnguyengtvthcm commented Aug 4, 2023

danwos left a comment

danwos Aug 7, 2023

nhatnamnguyengtvthcm Aug 7, 2023

danwos Aug 7, 2023

nhatnamnguyengtvthcm Aug 8, 2023

nhatnamnguyengtvthcm commented Aug 7, 2023

danwos left a comment •

edited by ubmarco

danwos Aug 7, 2023

nhatnamnguyengtvthcm Aug 8, 2023

danwos Aug 7, 2023

danwos commented Aug 7, 2023 •

edited

nhatnamnguyengtvthcm commented Aug 8, 2023 •

edited

danwos left a comment

danwos Aug 9, 2023

danwos commented Aug 24, 2023 •

edited

danwos left a comment

danwos Aug 25, 2023

danwos Aug 25, 2023

danwos left a comment

chrisjsewell left a comment

chrisjsewell commented Aug 28, 2023

nhatnamnguyengtvthcm commented Aug 28, 2023

nhatnamnguyengtvthcm commented Aug 28, 2023

nhatnamnguyengtvthcm commented Aug 28, 2023

danwos left a comment

danwos left a comment

r-o-b-e-r-t-o commented Sep 14, 2023

		@@ -131,6 +131,21 @@ def load_json(self, file) -> None:

		self.log.debug(f"needs.json file loaded: {file}")

		def write_jsons(self, needs_file, needs_path) -> None:

Namnn/need per id json #960

Namnn/need per id json #960

Conversation

nhatnamnguyengtvthcm commented Aug 4, 2023

danwos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nhatnamnguyengtvthcm commented Aug 7, 2023

danwos left a comment • edited by ubmarco

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danwos commented Aug 7, 2023 • edited

nhatnamnguyengtvthcm commented Aug 8, 2023 • edited

danwos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danwos commented Aug 24, 2023 • edited

danwos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danwos left a comment

Choose a reason for hiding this comment

chrisjsewell left a comment

Choose a reason for hiding this comment

chrisjsewell commented Aug 28, 2023

nhatnamnguyengtvthcm commented Aug 28, 2023

nhatnamnguyengtvthcm commented Aug 28, 2023

nhatnamnguyengtvthcm commented Aug 28, 2023

danwos left a comment

Choose a reason for hiding this comment

danwos left a comment

Choose a reason for hiding this comment

r-o-b-e-r-t-o commented Sep 14, 2023

danwos left a comment •

edited by ubmarco

danwos commented Aug 7, 2023 •

edited

nhatnamnguyengtvthcm commented Aug 8, 2023 •

edited

danwos commented Aug 24, 2023 •

edited