Feature/1 initial repository setup #2

miroslavpojer · 2024-05-09T09:50:29Z

Create the initial version of the project.
Populate by logic from the origin project and integrate.

Closes #1

… workflow files.

- Update of README.md. - Created alternative solution for current control sh file.

Zejnilovic · 2024-05-09T12:25:13Z

.github/workflows/check_pr_release_note_comment.yml

+              core.setFailed('No "Release notes" found in PR comments')
+            } else {
+              console.log('"Release notes" found in comments');
+            }


Suggested change

}

}

Fixed in commit - 0c435b7.

Zejnilovic · 2024-05-09T12:36:32Z

.github/workflows/release_draft.yml

+        uses: actions/github-script@v7
+        with:
+          script: |
+            const newTag = core.getInput('tag-name');


line 5 is tagName here it is tag-name. Why?

Fixed in commit - 0c435b7.

Zejnilovic · 2024-05-09T12:43:13Z

.github/workflows/release_notes_comments_migration.yml

+on:
+  pull_request:
+    types: [closed]
+    branches: [ master ]
+
+jobs:
+  copy_release_notes:
+    if: github.event.pull_request.merged == true


Why not on push to master?

The logic is to migrate all Release notes comments from PR to related Issues only when PR is closed.
By push, there would be repeated duplication.

Zejnilovic · 2024-05-09T12:45:36Z

.github/workflows/release_notes_comments_migration.yml

+                  body: commentBody
+                });            
+              }
+            }


Suggested change

}

}

Fixed in commit - 0c435b7.

Zejnilovic · 2024-05-09T12:50:58Z

.github/workflows/release_notes_comments_migration.yml

+        run: |
+          echo "Extracted Release Note Body:"
+          echo "${{ steps.get-comments.outputs.releaseNoteBody }}"
+          echo "RELEASE_NOTE_BODY<<EOF" >> $GITHUB_ENV


Is this maybe missing a =?

Zejnilovic · 2024-05-09T13:11:49Z

.github/workflows/release_notes_comments_migration.yml

+            const regexPattern = /([Cc]los(e|es|ed)|[Ff]ix(es|ed)?|[Rr]esolv(e|es|ed))\s*#\s*([0-9]+)/g;
+
+            let match;
+            while ((match = regexPattern.exec(description)) !== null) {


I have an issue to wrap my head around this one

This is a regex detector of the mentioned Issue in the PR description. It does not look nice, yes.

.github/workflows/unit_tests.yml

Zejnilovic · 2024-05-09T13:15:33Z

.gitignore

 #.idea/
+/.idea/


Addressed in commit - bcb84a5.

Zejnilovic · 2024-05-09T13:18:58Z

README.md

+- **Submit Pull Requests**: Feel free to fork the repository, make changes, and submit a pull request. Please ensure your code adheres to the existing style and all tests pass.
+- **Report Issues**: If you encounter any bugs or issues, please report them via the repository's [Issues page](https://github.com/AbsaOSS/living-doc-generator/issues).
+- **Suggest Enhancements**: Have ideas on how to make this action better? Open an issue to suggest enhancements.
+
+Before contributing, please review our [contribution guidelines](https://github.com/AbsaOSS/living-doc-generator/blob/master/CONTRIBUTING.md) for more detailed information.


Suggested change

- **Submit Pull Requests**: Feel free to fork the repository, make changes, and submit a pull request. Please ensure your code adheres to the existing style and all tests pass.

- **Report Issues**: If you encounter any bugs or issues, please report them via the repository's [Issues page](https://github.com/AbsaOSS/living-doc-generator/issues).

- **Suggest Enhancements**: Have ideas on how to make this action better? Open an issue to suggest enhancements.

Before contributing, please review our [contribution guidelines](https://github.com/AbsaOSS/living-doc-generator/blob/master/CONTRIBUTING.md) for more detailed information.

Before contributing, please review our [contribution guidelines](https://github.com/AbsaOSS/living-doc-generator/blob/master/CONTRIBUTING.md) for more detailed information.

Addressed in commit - bcb84a5.

- Not test run.

…dding py files into dist.

- Improving return of liv-doc root.

miroslavpojer · 2024-05-16T08:05:06Z

Release notes:

Empty project populated by all required project logic stuff.
Introduced Python code solution for data mining from GH repositories and Projects
Introduced GH Action logic to provide Python code to be used by GH Action usage.
Usage of action tested in the example project.

…It is failing.

MobiTikula · 2024-05-16T11:18:35Z

README.md

+See the default action step definition:
+
+```yaml
+- name: Generate Living Documentation


May change name to Living Documentation Generator everywhere in README as it is in action.yml file.

I have checked the file and fixed capital in 1st line. Other variations in the file still make sense to me.
Please provide reasons for the change in local meanings.

MobiTikula · 2024-05-16T11:29:05Z

dist/controller.py

+import re
+
+
+def extract_args():


Type hints and proper method / script documentation has to be added to the whole controller.py.

Please address this in the SPIKE issue as one of the goals.

MobiTikula · 2024-05-16T11:35:25Z

dist/controller.py

+    run_script('convert_features_to_pages.py', env_vars)
+
+
+if __name__ == '__main__':


Here is different pattern, than we usually have in our scripts, where we don't initialize main method. I would refactor the code so the scripts are consistent.

Please address this in the SPIKE issue as one of the goals.
This version aimed to provide a migrated solution that worked well enough to close previous projects.
No deep analysis was included, as this step prevented us from developing the final project.

lsulak · 2024-05-16T13:55:03Z

src/controller.py

+
+if __name__ == '__main__':
+    print("Starting Living Documentation generation.")
+    main()


This PR is seriously very big, almost no one will be able to review it fully in detail within reasonable amount of time. Better would be to create a few smaller ones. In your case, in particular, you could add one script at a time. One PR for clean_env_before_mining.py and related functionality, another for github_query_issues.py etc.

This version aimed to provide a migrated solution that worked well enough to close previous projects.
The Python scripts are the goal of that migration. We are working on collecting refactoring notes in parallel.

in a previous project

in a local project in SPIKE issue

Yes, it is big, but the script is the same. Only the main method and env variables loading differ.
All scripts need to be refactored. Adding a format check and using OOP will be the first steps; then, more reviews should follow.

lsulak · 2024-05-16T13:57:12Z

src/controller.py

+        env.update(env_vars)
+
+        # Running the script with updated environment
+        result = subprocess.run(['python3', script_name], env=env, text=True, capture_output=True, check=True)


calling Python from Python using subprocess is definitely not that common. Why not to call it directly? If env vars are the only reason, you can encapsulate them into a data class (I proposed something like that already in: #4 (comment)) and pass to each main function of each module, just an example.

See the solution proposal linked on following review comment.

lsulak · 2024-05-16T13:58:06Z

src/controller.py

+
+    # Clean environment before mining
+    print("Data mining for Living Documentation")
+    run_script('clean_env_before_mining.py', env_vars)


hardcoding python script name like this is definitely weird. I'd just import and call it directly as a standard Python call

I have prepared a solution proposal for the clean script - see commit 56c6fc9.
@MobiTikula, Can you address the refactoring of the rest of the scripts in SPIKE?

lsulak · 2024-05-16T14:01:33Z

src/consolidate_feature_data.py

+    # Set to store the names of repositories that have been used
+    set_of_used_repos = set()
+
+    if os.path.isdir(PROJECT_DIRECTORY):


this function is quite nested and I feel like almost a bit too long. Consider simplifying it and perhaps even break it down into multiple functions.

Btw, I like this approach to 'flatten' the code a bit - instead of:

if something: do_something_complex_and_nested

to:

if not something: return do_something_complex_and_nested

by following this second approach, your do_something_complex_and_nested code block is not nested any more :)

This is cool: https://peps.python.org/pep-0020/

Flat is better than nested.

As this was overtaken by migration, let's make this a note to refactor SPIKE @MobiTikula .

miroslavpojer added 3 commits May 9, 2024 09:48

- Initial files: issue templates, CODEOWNERS, CONTRIBUTING, RLS Notes…

bd92f4b

… workflow files.

- Added logic for GitHub action.

680821a

- Update of README.md. - Created alternative solution for current control sh file.

- Prepared Hello World solution of unit tests.

1257267

miroslavpojer self-assigned this May 9, 2024

Zejnilovic self-requested a review May 9, 2024 12:19

Zejnilovic reviewed May 9, 2024

View reviewed changes

miroslavpojer added 24 commits May 15, 2024 07:43

- Fix typos.

0c435b7

- Fix typos.

bcb84a5

- Initial implementation of Action.

e8bea67

- Not test run.

- Add built dist.

56cbf0a

- Fixing issues.

4ba0730

- Fixing wrong path from dist/index.js.

a3d68d7

- Improving amount of log information when error some.

cc7b0ee

- Adding missing require line.

dc3b0a9

- Adding more information messages.

f51f8ae

- Using core instead of console to log error details.

c877138

- Improve exec method.

4b49ae3

- Adding print to observe working dir.

f6b31c0

- Refactored the code structure to more fit with expected paths and a…

30c01ab

…dding py files into dist.

- Fixing path to python scripts to dist.

cce4930

- Fixing path to python scripts to '.'.

5723f04

- Fixing path to python scripts.

51b8be3

- Fixing path to python scripts.

9d28598

- Fixing path to python scripts.

5c0f3bb

- Fixed python paths. Removing additional logic.

edf236a

- Debugging solution. Adding info messages.

9d59a14

- Debugging solution. Adding info messages.

664ad15

- Keep camelCase for json values and keys.

673a49c

- Adding templates to solution.

34f4ef3

- Adding root dir for liv-doc.

7fa0509

- Improving return of liv-doc root.

miroslavpojer added 4 commits May 16, 2024 09:46

- Improving return of liv-doc root.

08fe603

- Improving return of liv-doc root.

241fbe8

- Improving return of liv-doc root.

5a6702e

- Improving return of liv-doc root.

71c0463

miroslavpojer marked this pull request as ready for review May 16, 2024 08:02

miroslavpojer added 3 commits May 16, 2024 10:06

- Fixed typo in test workflow.

28ad64b

- Fixed summy test to be passing again.

d17b05f

- Code coverage commented out as project does not contain real test. …

4619dc3

…It is failing.

miroslavpojer requested review from Zejnilovic, MobiTikula and lsulak May 16, 2024 09:03

MobiTikula reviewed May 16, 2024

View reviewed changes

lsulak reviewed May 16, 2024

View reviewed changes

miroslavpojer added 2 commits May 17, 2024 08:19

- Fix capital in first line of file.

e01b709

- Proposal of solution to remove hardcoded python script name.

56c6fc9

miroslavpojer requested review from MobiTikula and lsulak May 20, 2024 06:32

Zejnilovic approved these changes May 21, 2024

View reviewed changes

miroslavpojer merged commit c8121fb into master May 21, 2024
3 checks passed

miroslavpojer deleted the feature/1-Initial-repository-setup branch May 21, 2024 10:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/1 initial repository setup #2

Feature/1 initial repository setup #2

miroslavpojer commented May 9, 2024

Zejnilovic May 9, 2024

miroslavpojer May 15, 2024

Zejnilovic May 9, 2024

miroslavpojer May 15, 2024

Zejnilovic May 9, 2024

miroslavpojer May 15, 2024

Zejnilovic May 9, 2024

miroslavpojer May 15, 2024

Zejnilovic May 9, 2024

miroslavpojer May 16, 2024

Zejnilovic May 9, 2024

miroslavpojer May 15, 2024 •

edited

Loading

Zejnilovic May 9, 2024

miroslavpojer May 15, 2024

Zejnilovic May 9, 2024

miroslavpojer May 15, 2024

miroslavpojer commented May 16, 2024 •

edited

Loading

MobiTikula May 16, 2024 •

edited

Loading

miroslavpojer May 17, 2024

MobiTikula May 16, 2024

miroslavpojer May 17, 2024

MobiTikula May 16, 2024

miroslavpojer May 17, 2024

lsulak May 16, 2024

miroslavpojer May 17, 2024

lsulak May 16, 2024

miroslavpojer May 17, 2024

lsulak May 16, 2024

miroslavpojer May 17, 2024

lsulak May 16, 2024

lsulak May 16, 2024

miroslavpojer May 17, 2024

		run_script('convert_features_to_pages.py', env_vars)


		if __name__ == '__main__':

		#.idea/
		/.idea/

Feature/1 initial repository setup #2

Feature/1 initial repository setup #2

Conversation

miroslavpojer commented May 9, 2024

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

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

Choose a reason for hiding this comment

miroslavpojer May 15, 2024 • edited Loading

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

miroslavpojer commented May 16, 2024 • edited Loading

MobiTikula May 16, 2024 • edited Loading

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

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

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

miroslavpojer May 15, 2024 •

edited

Loading

miroslavpojer commented May 16, 2024 •

edited

Loading

MobiTikula May 16, 2024 •

edited

Loading