Skip to content

Comments

[Evil] EAPI -> EVIL_API#169

Merged
joaoantoniocardoso merged 3 commits intodevs/expertise/native-windowsfrom
devs/joaoantoniocardoso/native-windows/evil/missing-EAPI
Jun 25, 2020
Merged

[Evil] EAPI -> EVIL_API#169
joaoantoniocardoso merged 3 commits intodevs/expertise/native-windowsfrom
devs/joaoantoniocardoso/native-windows/evil/missing-EAPI

Conversation

@joaoantoniocardoso
Copy link
Contributor

@joaoantoniocardoso joaoantoniocardoso commented Jun 23, 2020

No description provided.

@joaoantoniocardoso joaoantoniocardoso self-assigned this Jun 23, 2020
JPTIZ
JPTIZ approved these changes Jun 23, 2020
View reviewed changes
Copy link
Contributor

@JPTIZ JPTIZ left a comment

Choose a reason for hiding this comment

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

Seems fine.

@joaoantoniocardoso joaoantoniocardoso changed the title [Evil] Fix missing EAPI [Evil] EAPI -> EVIL_API Jun 23, 2020
@joaoantoniocardoso joaoantoniocardoso requested a review from JPTIZ June 23, 2020 19:01
@felipealmeida
Copy link
Member

felipealmeida commented Jun 23, 2020

Tem como fazer um squash antes de mergear/rebase/cherry-pick ? Não faz sentido ficar com 2 commits

@joaoantoniocardoso
Copy link
Contributor Author

joaoantoniocardoso commented Jun 23, 2020

Tem como fazer um squash antes de mergear/rebase/cherry-pick ? Não faz sentido ficar com 2 commits

Sim, a gente tem usado squash and merge .

Coquinho
Coquinho reviewed Jun 24, 2020
View reviewed changes
src/lib/evil/evil_api.h Outdated
Comment on lines 4 to 5
#ifdef EVIL_API
#undef EVIL_API
Copy link
Contributor

@Coquinho Coquinho Jun 24, 2020

Choose a reason for hiding this comment

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

If EVIL_API is already defined shouldn't it yield an error? As it shouldn't be already defined?

Coquinho
Coquinho reviewed Jun 24, 2020
View reviewed changes
Copy link
Contributor

@Coquinho Coquinho left a comment

Choose a reason for hiding this comment

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

Shouldn't this evil_api.h use EVIL_STATIC and EVIL_BUiLD?

@joaoantoniocardoso
Copy link
Contributor Author

joaoantoniocardoso commented Jun 24, 2020

Shouldn't this evil_api.h use EVIL_STATIC and EVIL_BUILD?

Yeah, you're right, I'll change it to work in the same way of the other APIs.

@joaoantoniocardoso joaoantoniocardoso force-pushed the devs/joaoantoniocardoso/native-windows/evil/missing-EAPI branch from d149536 to 0cffb19 Compare June 24, 2020 13:40
@joaoantoniocardoso
Copy link
Contributor Author

joaoantoniocardoso commented Jun 24, 2020

Writing just to remember that #59 and #159 is adding new functions that this patch is not seeing yet.

@joaoantoniocardoso joaoantoniocardoso merged commit aac32eb into devs/expertise/native-windows Jun 25, 2020
JPTIZ
JPTIZ reviewed Jun 26, 2020
View reviewed changes
src/lib/evil/evil_api.h
Comment on lines +4 to 6
#ifdef EVIL_API
#error EVIL_API should not be already defined
#endif
Copy link
Contributor

@JPTIZ JPTIZ Jun 26, 2020

Choose a reason for hiding this comment

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

If we're using evil_api.h, there's no reason to check this at all actually, since the include-guards will handle it.

felipealmeida pushed a commit that referenced this pull request Oct 18, 2020
@joaoantoniocardoso @felipealmeida
[Evil] EAPI -> EVIL_API (#169)
5437e87 
* Renaming EAPI to EVIL_API

* Modified to use EVIL_BUILD
felipealmeida added a commit that referenced this pull request Oct 26, 2020
@felipealmeida
[Evil] EAPI -> EVIL_API (#169)
3f77101 
* Renaming EAPI to EVIL_API

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

Reviewers

@felipealmeida felipealmeida Awaiting requested review from felipealmeida

@cauebs cauebs Awaiting requested review from cauebs

@ricardocvel ricardocvel Awaiting requested review from ricardocvel

@Coquinho Coquinho Awaiting requested review from Coquinho

2 more reviewers

@JPTIZ JPTIZ JPTIZ approved these changes

@walac walac walac approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@joaoantoniocardoso joaoantoniocardoso

Labels

bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@joaoantoniocardoso @felipealmeida @walac @JPTIZ @Coquinho