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

update llvm.spec #65452

Closed
wants to merge 2 commits into from
Closed

update llvm.spec #65452

wants to merge 2 commits into from

Conversation

sunshaoce
Copy link
Contributor

No description provided.

@sunshaoce
Copy link
Contributor Author

The content of llvm.spec hasn't been updated for a long time, so I tried to update it.

pawosm-arm
pawosm-arm reviewed Sep 6, 2023
View reviewed changes
Copy link
Contributor

@pawosm-arm pawosm-arm left a comment

Choose a reason for hiding this comment

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

I'm surprised I was picked for that. How did you manage to reach me through this system? Don't we still use the Phabricator for doing reviews?

llvm/llvm.spec.in Outdated Show resolved Hide resolved
llvm/llvm.spec.in Outdated Show resolved Hide resolved
llvm/llvm.spec.in Show resolved Hide resolved
@sunshaoce
Copy link
Contributor Author

I'm surprised I was picked for that. How did you manage to reach me through this system? Don't we still use the Phabricator for doing reviews?

You have been automatically recommended as a reviewer by Github, and I am not sure what algorithm it is based on.

@pawosm-arm
Copy link
Contributor

pawosm-arm commented Sep 6, 2023 •
edited

There's something really disturbing about this file: it refers the configure script which is no longer usable for LLVM. I suppose this file can be removed entirely. I'd suggest adding @tstellar and @sylvestre as reviewers as they can give you more definite answer whether this file is still needed.

@tbaederr
Copy link
Contributor

tbaederr commented Sep 6, 2023

Who actually uses the llvm.spec?

@sylvestre
Copy link
Collaborator

I would just remove it

tstellar
tstellar requested changes Sep 12, 2023
View reviewed changes
Copy link
Collaborator

@tstellar tstellar left a comment

Choose a reason for hiding this comment

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

I agree with @sylvestre, I think it would be best to remove it. If you want a modern spec file for building RPMs, you can look at the Fedora Linux spec

@sunshaoce
Copy link
Contributor Author

Thanks for your replies!

@sunshaoce sunshaoce closed this Sep 12, 2023
@sunshaoce sunshaoce deleted the llvm_spec branch September 13, 2023 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pawosm-arm pawosm-arm pawosm-arm left review comments

@tstellar tstellar tstellar requested changes

@sylvestre sylvestre Awaiting requested review from sylvestre

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

Successfully merging this pull request may close these issues.

None yet
5 participants
@sunshaoce @pawosm-arm @tbaederr @sylvestre @tstellar