Skip to content

Conversation

johnaohara
Copy link
Member

…cs to 'enabledReports=true'

Reduces heap allocation and increase build speed of a default native image build

@johnaohara
Copy link
Member Author

This PR resolves #2241 but also retains current behaviour, i.e. if users currently define <disableReports>true</disableReports> report generation will be disabled

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Added a small question.

…cs to 'enabledReports=true'

Reduces heap allocation and increase build speed of a default native image build
@gsmet gsmet changed the title Disable native image PrintAnalysisCallTree by default, change semanti… Disable native image PrintAnalysisCallTree by default, change semantics to 'enabledReports=true' Oct 22, 2019
@gsmet gsmet added this to the 0.27.0 milestone Oct 22, 2019
@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 22, 2019
@Sanne
Copy link
Member

Sanne commented Oct 22, 2019

could we avoid the double negation? I almost misunderstood the option :)

I'd have <enableReports> an have it default to false.

@johnaohara
Copy link
Member Author

johnaohara commented Oct 22, 2019

@Sanne That is what I have changed it to. Previously to enable report generation, configuration was defined as;

<diableReports>false</disableReports>

whereas now, reports are not generated by default and report generation is enabled with;

<enableReports>true</enableReports>

I have retained the <disabledReports> configuration option to ensure existing builds that set that option do not fail

@gsmet gsmet merged commit 994427c into quarkusio:master Oct 22, 2019
@gsmet
Copy link
Member

gsmet commented Oct 22, 2019

Thanks @johnaohara !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage/waiting-for-ci Ready to merge when CI successfully finishes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants