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.

Sign up for GitHub

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

Jump to bottom

Change worfkow engine name in README.md files #5460

Merged
merged 3 commits into from
Jun 14, 2024
Merged

Change worfkow engine name in README.md files #5460

merged 3 commits into from
Jun 14, 2024

Conversation

mhamra
Copy link
Contributor

@mhamra mhamra commented Jun 4, 2024 •
edited
Loading

Description

Related issue: #5458

This PR changes the name of the workflow engine in the README.md files.

@mhamra mhamra linked an issue Jun 4, 2024 that may be closed by this pull request
Add DTT1 workflow tool name #5458
Closed
2 tasks
fcaffieri
fcaffieri requested changes Jun 5, 2024
View reviewed changes
Copy link
Member

@fcaffieri fcaffieri left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good Job, only there are still some references within:

deployability/README.MD Outdated
@@ -1,4 +1,4 @@
## Deployability general documentation
## JobFlow - Deployability general documentation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JobFlow is a module within deployability, we should remove it from here

Copy link
Contributor Author

@mhamra mhamra Jun 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in commit 8b07c42

deployability/modules/allocation/README.MD Outdated
@@ -1,12 +1,12 @@
## Allocation Module
## JobFlow - Allocation Module
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JobFlow is a module within deployability just like Allocation, we should remove it from here

Copy link
Contributor Author

@mhamra mhamra Jun 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in commit 8b07c42

deployability/modules/provision/README.MD Outdated
@@ -1,14 +1,14 @@
## Provision module
## JObFlow - Provision module
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JobFlow is a module within deployability just like Provision, we should remove it from here

Copy link
Contributor Author

@mhamra mhamra Jun 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in commit 8b07c42

deployability/modules/testing/README.MD Outdated
@@ -1,4 +1,4 @@
## Testing Module
## JobFlow - Testing Module
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JobFlow is a module within deployability just like Testing, we should remove it from here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in commit 8b07c42

rauldpm
rauldpm requested changes Jun 5, 2024
View reviewed changes
deployability/modules/provision/README.MD Outdated Show resolved Hide resolved
pro-akim
pro-akim requested changes Jun 10, 2024
View reviewed changes
Copy link
Member

@pro-akim pro-akim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GJ!, some small changes were requested.

deployability/modules/jobflow.egg-info/PKG-INFO Outdated Show resolved Hide resolved
deployability/modules/jobflow.egg-info/SOURCES.txt Outdated Show resolved Hide resolved
deployability/modules/jobflow.egg-info/dependency_links.txt Outdated Show resolved Hide resolved
deployability/modules/jobflow.egg-info/entry_points.txt Outdated Show resolved Hide resolved
deployability/modules/jobflow.egg-info/not-zip-safe Outdated Show resolved Hide resolved
deployability/modules/jobflow/tests/conftest.py Outdated Show resolved Hide resolved
deployability/modules/jobflow/tests/conftest.py Outdated Show resolved Hide resolved
deployability/modules/jobflow/tests/test_dag.py Outdated Show resolved Hide resolved
deployability/modules/jobflow/tests/test_workflow_file.py Outdated Show resolved Hide resolved
deployability/modules/jobflow/tests/test_workflow_processor.py Outdated Show resolved Hide resolved
@mhamra mhamra force-pushed the enhancement/5458-change-workflow-name-in-readme branch 3 times, most recently from 6596881 to 9946aab Compare June 11, 2024 14:39
pro-akim
pro-akim approved these changes Jun 11, 2024
View reviewed changes
Copy link
Member

@pro-akim pro-akim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Notes

LGTM

@rauldpm rauldpm requested review from fcaffieri and rauldpm June 11, 2024 15:13
rauldpm
rauldpm requested changes Jun 11, 2024
View reviewed changes
Copy link
Member

@rauldpm rauldpm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not going to review the rest of it because it is too repetitive

  • The name of the module is JobFlow, not Jobflow or jobflow
  • If you are referring to the module directory, a path, or the pip module, it is jobflow because it is coded like that
  • Do not use mass replace, there are a lot of double spaces between words
  • What happened to the .gitignore file? There is a workflow reference
  • Some sentences should be reviewed as jobflow does not match the context

deployability/README.MD Show resolved Hide resolved
deployability/README.MD Outdated Show resolved Hide resolved
deployability/README.MD Show resolved Hide resolved
deployability/README.MD Outdated Show resolved Hide resolved
deployability/README.MD Outdated Show resolved Hide resolved
deployability/modules/jobflow/README.MD Outdated Show resolved Hide resolved
deployability/modules/jobflow/README.MD Outdated Show resolved Hide resolved
deployability/modules/jobflow/README.MD Outdated Show resolved Hide resolved
deployability/modules/jobflow/README.MD Outdated Show resolved Hide resolved
deployability/modules/jobflow/README.MD Outdated Show resolved Hide resolved
@mhamra
Copy link
Contributor Author

mhamra commented Jun 11, 2024

Fixed .gitignore

wazuh-qa/.gitignore

Line 9 in 746eb38

deployability/modules/jobflow.egg-info

@mhamra mhamra force-pushed the enhancement/5458-change-workflow-name-in-readme branch from 746eb38 to e7ac71f Compare June 11, 2024 18:56
rauldpm
rauldpm requested changes Jun 12, 2024
View reviewed changes
Copy link
Member

@rauldpm rauldpm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are going to take another approach to this PR

  1. Revert all commits
  2. Change README titles
  3. Change the directory module from workflow_engine to jobflow
  4. Change all code that is affected by this directory change
  5. Review code and change Workflow (as name) to JobFlow, workflow is a word and has a meaning, take that into account

deployability/README.MD Show resolved Hide resolved
deployability/modules/jobflow/examples/agent/vagrant/test-agent-complete-1.yaml Show resolved Hide resolved
deployability/modules/jobflow/jobflow_processor.py Outdated Show resolved Hide resolved
deployability/modules/jobflow/jobflow_processor.py Outdated Show resolved Hide resolved
@mhamra mhamra force-pushed the enhancement/5458-change-workflow-name-in-readme branch from e7ac71f to 5d9d780 Compare June 13, 2024 21:20
@mhamra
Copy link
Contributor Author

mhamra commented Jun 13, 2024

UPDATE

I've implemented the requested changes of this comment in 5d9d780

@rauldpm rauldpm merged commit 7e1a276 into 4495-dtt1-release Jun 14, 2024
@rauldpm rauldpm deleted the enhancement/5458-change-workflow-name-in-readme branch June 14, 2024 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pro-akim pro-akim pro-akim approved these changes

@fcaffieri fcaffieri fcaffieri approved these changes

@rauldpm rauldpm rauldpm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add DTT1 workflow tool name
4 participants
@mhamra @rauldpm @fcaffieri @pro-akim