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.

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

Removing dependency for a specific gcc version #25

Merged
merged 2 commits into from
Apr 5, 2021

Conversation

hossein1387
Copy link
Contributor

@hossein1387 hossein1387 commented Mar 23, 2021

Description of PR that completes issue here...

Changelog

Fixed

  • Removed dependency in Makefile to use a specific gcc version

Added

  • Used only gcc and g++ commands so that the correct version is picked up by the build system

Changed

  • Used only gcc and g++ commands so that the correct version is picked up by the build system

Checklist

  • Automated tests pass
  • Changelog updated
  • Code style guideline is observed

Please check our contributing guidelines before opening a Pull Request.

@hossein1387 hossein1387 changed the title removing dependency to a specific gcc version Removing dependency for a specific gcc version Mar 23, 2021
Copy link
Contributor

@suehtamacv suehtamacv left a comment

Choose a reason for hiding this comment

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

Your changes look good to me, thanks for your contribution! Those are the compilers we use to compile GCC in our internal machines, this should not have been pushed to the open-source version of Ara.

I need a few stylistic changes before I can accept this PR. Could you please update the CHANGELOG file with your change? Also, we write our commit messages in the imperative tense, with a small tag indicating what does the commit change. For this commit, I imagine the [Makefile] tag would be appropriate.

@hossein1387
Copy link
Contributor Author

hossein1387 commented Mar 24, 2021

Your changes look good to me, thanks for your contribution! Those are the compilers we use to compile GCC in our internal machines, this should not have been pushed to the open-source version of Ara.

I need a few stylistic changes before I can accept this PR. Could you please update the CHANGELOG file with your change? Also, we write our commit messages in the imperative tense, with a small tag indicating what does the commit change. For this commit, I imagine the [Makefile] tag would be appropriate.

Ok got it, I will add a commit tag.

@mp-17
Copy link
Collaborator

mp-17 commented Mar 25, 2021

Your changes look good to me, thanks for your contribution! Those are the compilers we use to compile GCC in our internal machines, this should not have been pushed to the open-source version of Ara.
I need a few stylistic changes before I can accept this PR. Could you please update the CHANGELOG file with your change? Also, we write our commit messages in the imperative tense, with a small tag indicating what does the commit change. For this commit, I imagine the [Makefile] tag would be appropriate.

Ok got it, I will add a commit tag.

Hello Hossein, thanks a lot for your contribution.
The contribution guidelines are new, and maybe not completely clear: this is the first time we open the project to public, so please be patient with this commit stuff, I know it can be cumbersome :-)
The main purpose is to have a clean history, in which each commit is meaningful and consistent. Since in this pull request you have modified the Makefile and the Changelog, one possible log could be composed of 2 commits:

[Makefile] Remove dependency for a specific gcc version
[Changelog] Update changelog

Keeping the imperative form for the first verb, and avoiding merges from external branches. One useful tool to achieve this history from the current state can be git rebase -i.

Let us know if you need more information, and thanks again for the patience! ;-)

Copy link
Contributor

@suehtamacv suehtamacv left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your contribution @hossein1387!

@suehtamacv suehtamacv merged commit 021e783 into pulp-platform:main Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants