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

Change system_update.py and uyuni-tools/group_system_update.py for Pole #13

Merged
merged 6 commits into from
Oct 24, 2023

Conversation

mbrookhuis
Copy link
Contributor

Copy link
Member

@juliogonzalez juliogonzalez left a comment

Choose a reason for hiding this comment

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

I think you need to review all .idea stuff from all places. You added them to .gitignore, but they are still at the PR.

@admd admd requested a review from a team September 26, 2022 09:33
Copy link
Member

@rjmateus rjmateus left a comment

Choose a reason for hiding this comment

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

the folders "hubtools/.idea/.." and "uyuni-tools/.idea/.." still exists

@vzhestkov
Copy link

@mbrookhuis looks strange, but hubtools/.idea/ is still in the PR. And is the change of the file permission is really required in this PR?

@mbrookhuis
Copy link
Contributor Author

removed .idea.
Please merge

@juliogonzalez
Copy link
Member

removed .idea. Please merge

Please fix the permissions as requested by Viktor, as the PR is making executable (755) a lot files that don't need to be executable (yaml, txt, md...)

My guess is that only the .py script you are changing should be executable, and not sure about a couple of other .py scripts and the file general.

Copy link
Member

@juliogonzalez juliogonzalez left a comment

Choose a reason for hiding this comment

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

LGTM, as you can see I sent the fixes for the permissions on my own, so @mbrookhuis please review in case I broke something :-D

@rjmateus rjmateus merged commit c2f72ec into main Oct 24, 2023
@mbrookhuis mbrookhuis deleted the request_from_Pole_Emploi-MB20220505 branch May 16, 2024 14:01
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.

4 participants