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

Implement modification of %prep macros #52

Merged
merged 1 commit into from
May 13, 2022
Merged

Implement modification of %prep macros #52

merged 1 commit into from
May 13, 2022

Conversation

nforro
Copy link
Member

@nforro nforro commented Apr 24, 2022

Fixes #44.

@nforro nforro requested a review from FrNecas April 24, 2022 19:00
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

FrNecas
FrNecas reviewed Apr 29, 2022
View reviewed changes
Copy link
Contributor

@FrNecas FrNecas 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.

Just an initial review for now with some questions, it's a lot of changes to check. I will try to take one more look after the weekend with a fresh mind and being more familiar with the overall functionality :) The overall approach looks good to me

specfile/macro_options.py Show resolved Hide resolved
specfile/macro_options.py Show resolved Hide resolved
specfile/macro_options.py Show resolved Hide resolved
specfile/macro_options.py Outdated Show resolved Hide resolved
specfile/macro_options.py Show resolved Hide resolved
specfile/macro_options.py Show resolved Hide resolved
specfile/macro_options.py Show resolved Hide resolved
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ pre-commit SUCCESS in 1m 36s
✔️ specfile-tests-rpm-deps SUCCESS in 1m 02s
✔️ specfile-tests-pip-deps SUCCESS in 1m 06s

FrNecas
FrNecas approved these changes May 13, 2022
View reviewed changes
Copy link
Contributor

@FrNecas FrNecas left a comment

Choose a reason for hiding this comment

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

Basically one LGTM, I see one potentially unhandled edge case and maybe one typing error (though both of these may actually be correct, I am not sure).

specfile/macro_options.py Outdated Show resolved Hide resolved
FrNecas
FrNecas reviewed May 13, 2022
View reviewed changes
Copy link
Contributor

@FrNecas FrNecas left a comment

Choose a reason for hiding this comment

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

Eh, github for some reason didn't include this comment in the review, I hope that it will be sent now

specfile/macro_options.py Outdated Show resolved Hide resolved
@nforro
Implement modification of %prep macros
ce00d21 
This commit introduces `MacroOptions` class that allows for modification
of macro options while preserving whitespace and formatting.

Signed-off-by: Nikola Forró <nforro@redhat.com>
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ pre-commit SUCCESS in 1m 36s
✔️ specfile-tests-rpm-deps SUCCESS in 1m 02s
✔️ specfile-tests-pip-deps SUCCESS in 1m 02s

@nforro nforro added the mergeit Zuul, merge it! label May 13, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ pre-commit SUCCESS in 1m 35s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 2faf042 into main May 13, 2022
@delete-merged-branch delete-merged-branch bot deleted the nforro-prep branch May 13, 2022 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FrNecas FrNecas FrNecas approved these changes

Assignees
No one assigned
Labels
mergeit Zuul, merge it!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement modification of %prep macros
2 participants
@nforro @FrNecas