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

Let ijar warn only once per unsupported attribute #22072

Closed
wants to merge 3 commits into from
Closed

Let ijar warn only once per unsupported attribute #22072

wants to merge 3 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Apr 22, 2024

Work towards #21781

@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Apr 22, 2024
@fmeum fmeum requested a review from hvadehra April 22, 2024 13:47
@fmeum fmeum mentioned this pull request Apr 22, 2024
Open
hvadehra
hvadehra reviewed Apr 22, 2024
View reviewed changes
Copy link
Member

@hvadehra hvadehra left a comment

Choose a reason for hiding this comment

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

Thanks for this!

third_party/ijar/test/ijar_test.sh Outdated Show resolved Hide resolved
@hvadehra hvadehra requested a review from cushon April 22, 2024 14:13
@cushon
Copy link
Contributor

cushon commented Apr 22, 2024

Only warning once per attribute SGTM, thanks.

I think the idea behind the warnings is that we want to make a decision about new attributes, and either support it in ijar (if it's a standard JVM attribute that contains references to constant pool entries), or suppress the warning (if it's a proprietary attribute). For the osgi/aspectj ones in the bug, I think we could also add suppressions.

@fmeum
Copy link
Collaborator Author

fmeum commented Apr 22, 2024

CC @dkashyn-sfdc in case you want to submit a follow-up change to suppress these attributes.

@iancha1992 iancha1992 added the team-Rules-Java Issues for Java rules label Apr 22, 2024
@hvadehra hvadehra added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Apr 23, 2024
@dkashyn-sfdc
Copy link

@fmeum will this be released as part of 7.2?

@fmeum
Copy link
Collaborator Author

fmeum commented Apr 25, 2024

If I'm not mistaken, it will be in the next rules_java release, which you can update to independently of the next Bazel release (although it will also be the default with 7.2.0).

@dkashyn-sfdc
Copy link

Let's hope this got merged till 7.2.0 GA then ;)

@fmeum fmeum deleted the 21781-ijar-warning branch April 30, 2024 11:18
@sgowroji sgowroji removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label May 2, 2024
Kila2 pushed a commit to Kila2/bazel that referenced this pull request May 13, 2024
@fmeum @Kila2
Let ijar warn only once per unsupported attribute
c193f8b 
Work towards bazelbuild#21781

Closes bazelbuild#22072.

PiperOrigin-RevId: 629359080
Change-Id: I2c813d3a9069ec125a914bfc57e6c9068c83d3bc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cushon cushon cushon approved these changes

@hvadehra hvadehra hvadehra approved these changes

Assignees
No one assigned
Labels
team-Rules-Java Issues for Java rules
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@fmeum @cushon @dkashyn-sfdc @hvadehra @sgowroji @iancha1992