Remembering to push new changes with "autosquash" is one more thing to forget to do, just like forgetting to squash them was in the first place.
So I'm not sure this improves the situation unless we can require commits to a PR to be pushed in this way. (which would be yet another hook?) We want a hook that just says PRs with more than 1 commit cannot be merged. That prevents the error. ________________________________ From: Steve Lawrence <slawre...@apache.org> Sent: Thursday, December 12, 2019 7:17 AM To: dev@daffodil.apache.org <dev@daffodil.apache.org> Subject: Re: merge without squash - fix or leave it? Looking into the GitHub actions solution a bit, I found this GitHub action: https://github.com/xt0rted/block-autosquash-commits-action It's not exactly what I described, but when added to our config it will show a build failure (similar to a test failing) if a pull request has any *autosquash* commits. So one option would be to enable this and then adopt a workflow where commits added to a pull request should be created with either "git commit --fixup HEAD" or "git commit --squash HEAD". This will mark the new commit as an autosquash commit and it will be more visible when it's time to merge. Thoughts? On 12/11/19 8:08 AM, Steve Lawrence wrote: > Considering our user base is still fairly small and you just committed > it, I don't think it's unreasonable to force push to have a clean git > commit history. > > But as the project grows, this is something we'll likely want to avoid. > > I think there's a way to use the new github actions feature to add > custom checks before allowing a merge, similar to the CI stuff we do > now. To prevent this in the future, I imagine it wouldn't be hard to add > a check that won't allow merging if there are more than one commit in > the PR. > > On 12/11/19 8:02 AM, Beckerle, Mike wrote: >> I neglected to squash the two commits together before merging the >> daffodil-2242-tunable branch, which is the standard for our workflow. >> >> Should I fix and force push, or just leave it? I.e., which is the greater >> sin, to not squash and litter the history, or force push to master? >> >> ________________________________________ >> From: mbecke...@apache.org <mbecke...@apache.org> >> Sent: Wednesday, December 11, 2019 7:54 AM >> To: comm...@daffodil.apache.org >> Subject: [incubator-daffodil] 02/02: Fix 2.11 scala compile issue. >> >> This is an automated email from the ASF dual-hosted git repository. >> >> mbeckerle pushed a commit to branch master >> in repository https://gitbox.apache.org/repos/asf/incubator-daffodil.git >> >> commit 7aaabca399367b59f1eb36503d8510ed71d1e11b >> Author: Michael Beckerle <mbecke...@tresys.com> >> AuthorDate: Tue Dec 10 13:19:37 2019 -0500 >> >> Fix 2.11 scala compile issue. >> >> Recursive definition needed type in 2.11. Somehow 2.12 does without >> this. >> >> DAFFODIL-2242 >> --- >> .../src/main/scala/org/apache/daffodil/dpath/Expression.scala | 10 >> ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git >> a/daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala >> b/daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala >> index eb135fd..fcf666f 100644 >> --- a/daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala >> +++ b/daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala >> @@ -39,6 +39,7 @@ import org.apache.daffodil.BasicComponent >> import org.apache.daffodil.api.DaffodilTunables >> import org.apache.daffodil.oolag.OOLAG.OOLAGHostImpl >> import org.apache.daffodil.oolag.OOLAG.OOLAGHost >> +import org.apache.daffodil.api.UnqualifiedPathStepPolicy >> >> /** >> * Root class of the type hierarchy for the AST nodes used when we >> @@ -62,8 +63,8 @@ abstract class Expression extends OOLAGHostImpl() >> requiredEvaluations(isTypeCorrect) >> requiredEvaluations(compiledDPath_) >> >> - override lazy val tunable = parent.tunable >> - override lazy val unqualifiedPathStepPolicy = >> parent.unqualifiedPathStepPolicy >> + override lazy val tunable: DaffodilTunables = parent.tunable >> + override lazy val unqualifiedPathStepPolicy: UnqualifiedPathStepPolicy = >> parent.unqualifiedPathStepPolicy >> /** >> * Override where we traverse/access elements. >> */ >> @@ -575,8 +576,9 @@ case class WholeExpression( >> host: BasicComponent) >> extends Expression { >> >> - final override lazy val tunable = host.tunable >> - final override lazy val unqualifiedPathStepPolicy = >> host.unqualifiedPathStepPolicy >> + final override lazy val tunable: DaffodilTunables = host.tunable >> + final override lazy val unqualifiedPathStepPolicy : >> UnqualifiedPathStepPolicy >> + = host.unqualifiedPathStepPolicy >> >> def init() { >> this.setOOLAGContext(host) // we are the root of expression, but we >> propagate diagnostics further. >> >> >
