-
Notifications
You must be signed in to change notification settings - Fork 173
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
Document workflow based on the discussion in #5 #94
Conversation
I think item 5 is an absolute must, but I frankly think that item 4 should be accompanied by at least a draft of the specification. That is the API after all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it, and think this is a good starting point that captures the general themes and ideas discussed in #5.
A few questions/food for thought:
- The canonical name for instructions on how to contribute to a repository is
CONTRIBUTING.md
. Should this document be renamed? - Where do more mechanical/logistical workflow considerations get documented on the level of:
- [Github flow] vs [GitFlow] branching & release workflow/models
- Is it OK to (and when is it OK to):
- Squash merge
- Rebase merge
- Fast forward merge
Basically, I think we should have a CONTRIBUTING.md
and need to figure out at some point:
- How is this document distinct, and, if so, what is each document responsible for
- Additional, more logistic workflow/contributing details like fork vs branch, etc.
@nncarlson I like your idea that already at step 4. there should be a draft of the specification. What we can do is to write the specification for |
Thanks for opening this PR, and I agree we should require a spec at step 4. I skimmed through and like what I see so far, but will give detailed read-through and suggestions later tonight. |
I updated the document to include the specification requirement already in step 3. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this workflow a lot. I have a few content and wording suggestions. I list them all here because I can't quite get the suggestions to workout for multiline edits.
First, I suggest adding a statement at the beginning that would invite and encourage newcomers (to either stdlib or Fortran) to participate, something like:
"We welcome everyone and anyone to participate and propose additions to stdlib. It's okay if you don't have experience for specification or implementation, but have an idea for stdlib. If the idea is popular among the community, more experienced contributors will help it through all 5 steps."
Use a keyword for each step to make it more memorable. Something like:
- Idea: ...
- API: ...
- Specification: ...
- Implementation: ...
- Stabilize: ...
Third, add a sentence to the end of each Step 1 and 2 to describe the key goal of the step (other steps are clear!), I suggest:
- ... The goal of this step is to find out if the community is interested in having this functionality as part of stdlib.
- ... This step is exploratory and its goal is to find out what should the API look and feel like.
In Step 5 I suggest using the word "stable" instead of "main". (now you see why I called Step 5 Stabilize)
In Step 4, I suggest instead of this:
We should not merge if there is a strong objection from the reviewers or from somebody in the wider community.
To say:
"We can merge when there is majority approval (4/5) of the PR."
Rationale: We shouldn't have any one person be able to block the PR from going forward if they strongly object. If I don't like the API or implementation but 4 other contributors strongly support it (because to successfully work through Steps 2 and 3 you have to strongly support it), I shouldn't be able to throw a wrench in. Majority support should be good enough for merge here. Step 3 is already quite strict as it requires a majority support for a spec, so in Step 4 we are likely to disagree only about internal implementation.
Please adopt these edits if you like them. I approve this PR either way.
Mostly agree. Instead of stabilization, why not to call it standardization? And experimental vs standard.
…On Mon, Jan 6, 2020, at 5:09 PM, Milan Curcic wrote:
***@***.**** approved this pull request.
I like this workflow a lot. I have a few content and wording
suggestions. I list them all here because I can't quite get the
suggestions to workout for multiline edits.
First, I suggest adding a statement at the beginning that would invite
and encourage newcomers (to either stdlib or Fortran) to participate,
something like:
"We welcome everyone and anyone to participate and propose additions to
stdlib. It's okay if you don't have experience for specification or
implementation, but have an idea for stdlib. If the idea is popular
among the community, more experienced contributors will help it through
all 5 steps."
Use a keyword for each step to make it more memorable. Something like:
1. *Idea*: ...
2. *API*: ...
3. *Specification*: ...
4. *Implementation*: ...
5. *Stabilize*: ...
Third, add a sentence to the end of each Step 1 and 2 to describe the
key goal of the step (other steps are clear!), I suggest:
1. ... The goal of this step is to find out if the community is
interested in having this functionality as part of stdlib.
2. ... This step is exploratory and its goal is to find out what
should the API *look* and *feel* like.
In Step 5 I suggest using the word "stable" instead of "main". (now you
see why I called Step 5 Stabilize)
In Step 4, I suggest instead of this:
> We should not merge if there is a strong objection from the reviewers or from somebody in the wider community.
To say:
"We can merge when there is majority approval (4/5) of the PR."
Rationale: We shouldn't have any one person be able to block the PR
from going forward if they strongly object. If I don't like the API or
implementation but 4 other contributors strongly support it (because to
successfully work through Steps 2 and 3 you have to strongly support
it), I shouldn't be able to throw a wrench in. Majority support should
be good enough for merge here. Step 3 is already quite strict as it
requires a majority support for a spec, so in Step 4 we are likely to
disagree only about internal implementation.
Please adopt these edits if you like them. I approve this PR either way.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#94?email_source=notifications&email_token=AAAFAWGGY4QGQKPSXKXU2QTQ4PB4XA5CNFSM4KDI2ML2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCQ2ECBI#pullrequestreview-338968837>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWH7ESG62ACZIS3OSATQ4PB4XANCNFSM4KDI2MLQ>.
|
I think "standard" may be confusing, especially if we use the word "Standardize". It sounds a bit too close to "become part of the Fortran Standard". But that's not what we mean because we hope that some of the functions eventually transition from stdlib to the Standard. That would indeed be to "Standardize". Now, if we mean "standard" as "accepted as part of the standard library", then it implies that experimental is somehow non-standard, or not part of stdlib, which I think it is. I think it's just inconvenient jargon that there is both a standard and stdlib but they mean different things. What we're really doing here is transition from what everybody else calls "development" -> "stable". |
I think "Standardize" should be Step 6, 5-10 years in the future :) |
How would you describe our "stable" section of stdlib?
I think we do more than just a regular stable branch from other projects.
…On Mon, Jan 6, 2020, at 5:52 PM, Milan Curcic wrote:
I think "Standardize" should be Step 6, 5-10 years in the future :)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#94?email_source=notifications&email_token=AAAFAWEL7CUBEZBMQRGF2ETQ4PG4FA5CNFSM4KDI2ML2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIHJUEQ#issuecomment-571382290>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWC3TDRGPZPDYJX36DDQ4PG4FANCNFSM4KDI2MLQ>.
|
For example we will require specification document, which other projects do not.
…On Mon, Jan 6, 2020, at 6:57 PM, Ondřej Čertík wrote:
How would you describe our "stable" section of stdlib?
I think we do more than just a regular stable branch from other projects.
On Mon, Jan 6, 2020, at 5:52 PM, Milan Curcic wrote:
> I think "Standardize" should be Step 6, 5-10 years in the future :)
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#94?email_source=notifications&email_token=AAAFAWEL7CUBEZBMQRGF2ETQ4PG4FA5CNFSM4KDI2ML2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIHJUEQ#issuecomment-571382290>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWC3TDRGPZPDYJX36DDQ4PG4FANCNFSM4KDI2MLQ>.
>
|
How about "release"? It's been in experimental and now blessed for broad use. |
I'd describe a feature in stable or main as:
|
I like release also. |
I am going to go ahead and merge this, as it is good enough, so that we can move on. Let's submit further PRs if somebody has any ideas for improvement. |
Squashed commit of the following: commit 3266163 Merge: e96c12d 4274f0d Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 20:47:32 2020 +0100 modification of CMake and Makefile Merge branch 'stat_cmake' into stat_dev commit 4274f0d Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 20:44:24 2020 +0100 stat_cmake: update Makefile commit 17e3d16 Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 20:35:19 2020 +0100 second try cmake commit 397eb18 Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 20:18:37 2020 +0100 Modifications of CMake for tests on Ubuntu 7 commit e96c12d Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 19:40:05 2020 +0100 small change in md commit 7eec9ae Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 19:37:06 2020 +0100 stat_dev: renamed stat to stats commit 8199b6d Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 19:26:20 2020 +0100 stat_dev: changed spec commit b1c481d Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 19:15:25 2020 +0100 stat_dev: modifs following comments commit e64657c Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 14:23:59 2020 +0100 stat_dev: addition of .md file for mean commit ad504e8 Merge: 5a1adcb bab50e3 Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 13:16:21 2020 +0100 Merge remote-tracking branch 'jvdp1/stat_dev_1' into stat_dev commit bab50e3 Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 13:13:00 2020 +0100 stat_dev_1: changed all to iterations commit 8d4c11f Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 10:31:26 2020 +0100 stat_dev_1:moved all calls to mean functions to loops commit 922e523 Author: Vandenplas, Jeremie <[email protected]> Date: Tue Jan 21 09:13:10 2020 +0100 stat_dev_1: update test_mean commit 5a1adcb Author: Vandenplas, Jeremie <[email protected]> Date: Mon Jan 20 23:55:15 2020 +0100 stat_dev: inverting loops for efficiency commit 86970ae Author: Vandenplas, Jeremie <[email protected]> Date: Mon Jan 20 22:54:55 2020 +0100 stat_dev: use specific interface commit 6574a67 Author: Vandenplas, Jeremie <[email protected]> Date: Mon Jan 20 22:36:33 2020 +0100 stat_dev: addition of calls to error_stop commit e98090b Author: Vandenplas, Jeremie <[email protected]> Date: Mon Jan 20 21:32:27 2020 +0100 stat_dev: extension to rank 15 commit e0e3092 Author: Vandenplas, Jeremie <[email protected]> Date: Mon Jan 20 12:46:44 2020 +0100 stat_dev: simplified merge commit 22ff6e4 Author: Vandenplas, Jeremie <[email protected]> Date: Mon Jan 20 10:38:41 2020 +0100 stat_dev: progress rank 3 commit 7612613 Author: Vandenplas, Jeremie <[email protected]> Date: Mon Jan 20 10:34:06 2020 +0100 stat_dev: add rank 3 commit 60ab523 Author: Vandenplas, Jeremie <[email protected]> Date: Sun Jan 19 22:14:51 2020 +0100 stat_dev: addition of integer cases commit 6fb6ca5 Author: Vandenplas, Jeremie <[email protected]> Date: Sun Jan 19 21:03:46 2020 +0100 stat_dev: avoid allocatable functions commit a1c6353 Author: Vandenplas, Jeremie <[email protected]> Date: Sun Jan 19 20:11:49 2020 +0100 modification to have the same behaviour as Fortran sum commit 72500e1 Author: Vandenplas, Jeremie <[email protected]> Date: Sun Jan 19 15:23:34 2020 +0100 stat_dev: add error_stop commit 1272574 Author: Vandenplas, Jeremie <[email protected]> Date: Sun Jan 19 15:02:52 2020 +0100 stat_dev: update Makefile commit 426d43f Author: Vandenplas, Jeremie <[email protected]> Date: Sun Jan 19 12:21:09 2020 +0100 stat_dev: addition of test and creation of modules and submodules with fypp how to use pure functions inside submodules commit 965f37b Author: Vandenplas, Jeremie <[email protected]> Date: Sun Jan 19 11:35:11 2020 +0100 moved to submodules how to use pure functions in submodules commit d9af336 Author: Vandenplas, Jeremie <[email protected]> Date: Sun Jan 19 11:22:52 2020 +0100 stat_dev: init commit dc7e49b Merge: f300f4a cb7cf71 Author: Ondřej Čertík <[email protected]> Date: Tue Jan 14 11:11:12 2020 -0700 Merge pull request fortran-lang#109 from nncarlson/target-include Update CMakeLists handling of .mod files commit cb7cf71 Author: Neil Carlson <[email protected]> Date: Mon Jan 13 16:38:12 2020 -0700 Update CMakeLists handling of .mod files commit f300f4a Merge: 12bd060 0ea0ee1 Author: Milan Curcic <[email protected]> Date: Wed Jan 8 18:20:09 2020 -0500 Merge pull request fortran-lang#54 from scivision/systemlib add system module commit 0ea0ee1 Author: Michael Hirsch, Ph.D <[email protected]> Date: Mon Jan 6 11:41:46 2020 -0500 make sleep test automated check with system_clock commit c8974dc Author: Michael Hirsch, Ph.D <[email protected]> Date: Mon Dec 30 16:20:55 2019 -0500 add system module There are a number of capabilities it would be useful to bring from cstdlib and STL. This is an initial demonstration, replacing the non-cross-compiler sleep() with a standard implmeentation that works across compilers and operating systems, with millisecond integer input. commit 12bd060 Merge: 006beda e1d861d Author: Ondřej Čertík <[email protected]> Date: Wed Jan 8 11:52:06 2020 -0700 Merge pull request fortran-lang#97 from certik/goals Add Goals and Motivation section into README commit e1d861d Author: Ondřej Čertík <[email protected]> Date: Wed Jan 8 10:00:27 2020 -0700 Update README.md commit ca4554a Author: Ondřej Čertík <[email protected]> Date: Wed Jan 8 09:49:39 2020 -0700 Add Goals and Motivation section into README commit 006beda Merge: 1926ade e81d295 Author: Ondřej Čertík <[email protected]> Date: Tue Jan 7 15:41:39 2020 -0700 Merge pull request fortran-lang#94 from certik/workflow Document workflow based on the discussion in #5 commit e81d295 Author: Ondřej Čertík <[email protected]> Date: Tue Jan 7 08:20:24 2020 -0700 Update the workflow based on feedback commit 1926ade Merge: 7a6108e f857482 Author: Ondřej Čertík <[email protected]> Date: Tue Jan 7 08:01:39 2020 -0700 Merge pull request fortran-lang#96 from nshaffer/dev-optval Make optval pure or pure elemental where possible commit f857482 Merge: 274a2bb 7a6108e Author: Ondřej Čertík <[email protected]> Date: Tue Jan 7 07:48:17 2020 -0700 Merge branch 'master' into dev-optval commit 274a2bb Author: Nathaniel Shaffer <[email protected]> Date: Tue Jan 7 07:00:21 2020 -0700 add tests for 1d arrays (reals, ints, logical) commit e06e322 Author: Nathaniel Shaffer <[email protected]> Date: Tue Jan 7 06:58:14 2020 -0700 add "elemental" and/or "pure" attributes where possible commit 92926e0 Author: Ondřej Čertík <[email protected]> Date: Mon Jan 6 15:01:31 2020 -0700 Make the specification requirement part of step 3. commit 1f56d0d Author: Ondřej Čertík <[email protected]> Date: Mon Jan 6 12:09:48 2020 -0700 Document workflow based on the discussion in #5 commit 7a6108e Merge: e2b0cda a606606 Author: Izaak "Zaak" Beekman <[email protected]> Date: Mon Jan 6 13:02:55 2020 -0500 ci ctest enhancements (fortran-lang#92) Merge [scivision:citime] into master [scivision:citime]: https://github.com/scivision/stdlib/tree/citime commit a606606 Author: Michael Hirsch, Ph.D <[email protected]> Date: Mon Jan 6 11:50:17 2020 -0500 ci ctest enhancements commit e2b0cda Merge: 57d99f8 f0a6886 Author: Ondřej Čertík <[email protected]> Date: Mon Jan 6 08:01:24 2020 -0700 Merge pull request fortran-lang#90 from certik/stream Use access = "stream" by default commit f0a6886 Author: Ondřej Čertík <[email protected]> Date: Mon Jan 6 07:56:18 2020 -0700 Update src/stdlib_experimental_io.f90 Co-Authored-By: Jeremie Vandenplas <[email protected]> commit 05540fd Author: Ondřej Čertík <[email protected]> Date: Mon Jan 6 07:52:24 2020 -0700 Use access = "stream" by default commit 57d99f8 Merge: c3e4816 d845f2d Author: Ondřej Čertík <[email protected]> Date: Mon Jan 6 07:42:42 2020 -0700 Merge pull request fortran-lang#89 from pdebuyl/qsavetxt_format_string Use explicit formatting in qsavetxt commit d845f2d Author: Pierre de Buyl <[email protected]> Date: Mon Jan 6 10:35:55 2020 +0100 Use explicit formatting in qsavetxt
No description provided.