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

Refactor the manual Makefiles #70

Merged
merged 3 commits into from
Jan 3, 2020
Merged

Refactor the manual Makefiles #70

merged 3 commits into from
Jan 3, 2020

Conversation

certik
Copy link
Member

@certik certik commented Jan 2, 2020

  • Compile the quadruple precision loadtxt
  • Implement "make test" to run tests and run them at the CI
  • Create a static libstdlib.a library and use it in tests
  • Use more modern syntax and simplify
  • Pass in the FC/FCFLAGS variables automatically
  • Consolidate stdlib tests targets

@certik
Copy link
Member Author

certik commented Jan 2, 2020

Further simplifications can be made how the tests are handled to reduce code duplication in individual test directories (loadtxt and ascii), but we can do that later. The current design in this PR greatly simplifies the necessary changes to manual Makefiles required as part of #63 and #53.

@certik certik requested review from zbeekman and milancurcic January 2, 2020 22:47
@certik
Copy link
Member Author

certik commented Jan 3, 2020

@gronki, as another proponent of manual Makefiles (per your comments at j3-fortran/fortran_proposals#104 (comment)), I was wondering if you could please review this PR. I tried to simplify the Makefiles a lot.

milancurcic
milancurcic approved these changes Jan 3, 2020
View reviewed changes
Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

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

All looks great. Better than my Makefiles :D

@certik
Copy link
Member Author

certik commented Jan 3, 2020

Thanks @milancurcic for the review. I am rebasing on top of the latest master.

@gronki
Copy link

gronki commented Jan 3, 2020

@certik I only would suggest using FFLAGS for Fortran flags. I know there was an argument that FCFLAGS corresponds to f90+, but I think we should assume there is only one Fortran, and FFLAGS is clearly stated in gnu make manual as Fortran flags and followed by most Linux operating systems packages (such as rpmbuild that I use). I also think that .SUFFIXES is obsolete and not needed with new syntax using %. The rest is perfect!

@certik
Refactor the manual Makefiles
a222c6a 
* Compile the quadruple precision loadtxt
* Implement "make test" to run tests and run them at the CI
* Create a static libstdlib.a library and use it in tests
* Use more modern syntax and simplify
* Pass in the FC/FCFLAGS variables automatically
* Consolidate stdlib tests targets
@certik
Copy link
Member Author

certik commented Jan 3, 2020

@gronki thanks for the review! I fixed both things in 0d80b89 and 3400515.

I am going to merge it now, and we can further improve upon this later.

@certik certik merged commit 339299c into fortran-lang:master Jan 3, 2020
@certik certik deleted the makefiles2 branch January 3, 2020 18:31
zbeekman
zbeekman reviewed Jan 3, 2020
View reviewed changes
Copy link
Member

@zbeekman zbeekman left a comment

Choose a reason for hiding this comment

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

LGTM

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

@zbeekman zbeekman zbeekman left review comments

@milancurcic milancurcic milancurcic approved these changes

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

Successfully merging this pull request may close these issues.
4 participants
@certik @gronki @zbeekman @milancurcic