You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by Volker Lamp <vl...@apache.org> on 2022/05/31 20:56:27 UTC

Thoughts for 5.8.2

Hi guys

Pickung up what Thiago's wrote in another thread recently -

> What about a release next week? I've just committed CORS support and 
> I'm looking forward to get it released. :) 

Here are my thoughts about the next release. Well done for adding CORS 
support, Thiago. It's a useful addition to Tapestry. Further, we have a 
number of smaller bug fixes and improvements. I'm all for releasing 
early, however, I think we should make an effort towards including the 
fixes for the issues listed below also:

TAP5-2718 Tapestry Announcement lacks version

TAP5-2714 Checkbox component with Bootstrap 4

TAP5-2721 Broken links to 5.8.1 asc files on download
           page (more a todo than a commit)

The above are probably not very complicated or challenging. Fixing them 
can make a difference for new users, though  - no second chance for a 
first impression.


TAP5-2713 ChecksumPath
I understand Ben already fixed this one, so the issue could be set to 
closed also.

TAP5-2723 CronExpression fails handling L-<dayOfMonth>
It would be nice if Charles didn't have to wait another few months for a 
fix. That said, I agree with Ben: the code in CronExpression isn't easy 
to understand and hence the cause of the problem not easily spotted. My 
small contribution so far is a test case which I added to the TAP5-2723 
branch. I won't have the time to work on it more during the next couple 
of weeks, so either somebody else picks it up or we just leave it until 
later.

Thanks,

Volker




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by Volker Lamp <vl...@apache.org>.
> Updating Tapestry's CronExpression with the latest one from Quartz
> makes the test added by Volker (thanks again!) pass locally. I've
> committed the change and set the ticket to Resolved.

Great you found the time fixing it before the next release, Thiago.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by "Thiago H. de Paula Figueiredo" <th...@gmail.com>.
On Tue, May 31, 2022 at 10:18 PM Thiago H. de Paula Figueiredo
<th...@gmail.com> wrote:
>
> On Tue, May 31, 2022 at 5:57 PM Volker Lamp <vl...@apache.org> wrote:
> > TAP5-2723 CronExpression fails handling L-<dayOfMonth>

Updating Tapestry's CronExpression with the latest one from Quartz
makes the test added by Volker (thanks again!) pass locally. I've
committed the change and set the ticket to Resolved.

-- 
Thiago

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by Ben Weidig <be...@netzgut.net>.
Hi,

no objections from me.
Looking forward to the release!

Cheers
Ben


On Wed, Jun 15, 2022 at 9:10 AM Volker Lamp <vl...@apache.org> wrote:

> Hi Thiago
>
> > With TAP5-2725 sorted out, I'm planning on starting the release
> > process for 5.8.2 tomorrow. Any objections?
>
> Let's do it!
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
>

Re: Thoughts for 5.8.2

Posted by Volker Lamp <vl...@apache.org>.
Hi Thiago

> With TAP5-2725 sorted out, I'm planning on starting the release
> process for 5.8.2 tomorrow. Any objections?

Let's do it!


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by "Thiago H. de Paula Figueiredo" <th...@gmail.com>.
Hello, team!

With TAP5-2725 sorted out, I'm planning on starting the release
process for 5.8.2 tomorrow. Any objections?

Cheers!

On Fri, Jun 10, 2022 at 10:52 AM Thiago H. de Paula Figueiredo
<th...@gmail.com> wrote:
>
> On Thu, Jun 9, 2022 at 1:32 PM Volker Lamp <vl...@apache.org> wrote:
>
> > Hello Thiago
>
> Hi!
>
> > > I've also published a 5.8.2 snapshot to the ASF snapshots repository
> > > with the latest code on master.
> >
> > Nice! I'm testing it with two different Tapestry apps - no complaints so far.
>
> Awesome! Thanks for testing!
>
> > How about inviting users to plug it into their apps to get feedback from more people? A brief description of how to add the snapshot repo to their Maven/Gradle could be useful as well.
>
> That's a good idea, but I believe it would be against ASF policies.
> According to my reading of
> https://www.apache.org/legal/release-policy.html#publication, we
> cannot publicize snapshots in the users mailing list: "Projects SHALL
> publish official releases and SHALL NOT publish unreleased materials
> outside the development community." I suppose "development community"
> here mostly means the dev and private mailing lists. The users mailing
> list would be the Tapestry community (i.e. users and contributors),
> while contributors (i.e. committers and people who provide patches and
> ideas) would be the Tapestry development community.
>
> I can ask for a clarification. Not sure when I'll get an answer.
>
> Cheers!
>
> --
> Thiago



-- 
Thiago

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by "Thiago H. de Paula Figueiredo" <th...@gmail.com>.
On Thu, Jun 9, 2022 at 1:32 PM Volker Lamp <vl...@apache.org> wrote:

> Hello Thiago

Hi!

> > I've also published a 5.8.2 snapshot to the ASF snapshots repository
> > with the latest code on master.
>
> Nice! I'm testing it with two different Tapestry apps - no complaints so far.

Awesome! Thanks for testing!

> How about inviting users to plug it into their apps to get feedback from more people? A brief description of how to add the snapshot repo to their Maven/Gradle could be useful as well.

That's a good idea, but I believe it would be against ASF policies.
According to my reading of
https://www.apache.org/legal/release-policy.html#publication, we
cannot publicize snapshots in the users mailing list: "Projects SHALL
publish official releases and SHALL NOT publish unreleased materials
outside the development community." I suppose "development community"
here mostly means the dev and private mailing lists. The users mailing
list would be the Tapestry community (i.e. users and contributors),
while contributors (i.e. committers and people who provide patches and
ideas) would be the Tapestry development community.

I can ask for a clarification. Not sure when I'll get an answer.

Cheers!

--
Thiago

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by Volker Lamp <vl...@apache.org>.
Hello Thiago

> I've also published a 5.8.2 snapshot to the ASF snapshots repository
> with the latest code on master.

Nice! I'm testing it with two different Tapestry apps - no complaints so far.

How about inviting users to plug it into their apps to get feedback from more people? A brief description of how to add the snapshot repo to their Maven/Gradle could be useful as well.

Thanks,

Volker

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by "Thiago H. de Paula Figueiredo" <th...@gmail.com>.
I've also published a 5.8.2 snapshot to the ASF snapshots repository
with the latest code on master.

On Tue, Jun 7, 2022 at 9:35 PM Thiago H. de Paula Figueiredo
<th...@gmail.com> wrote:
>
> On Tue, Jun 7, 2022 at 8:31 AM Volker Lamp <vl...@apache.org> wrote:
>
> > TAP5-2718 (Tapestry Announcement lacks version) is still showing 'UNKNOWN'. I reopened the issue and pushed a test case to a new branch.
>
> Thanks for testing it! Yeah, it wasn't fixed properly, but now it is.
>
> > TAP5-2721 Broken links to 5.8.1 asc files on download page
> > If it's a "can't fix" the ticket should be decorated :) with a comment and then closed, so that the reporter receives feedback.
>
> I just did exactly that. :)
>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> > For additional commands, e-mail: dev-help@tapestry.apache.org
> >
>
>
> --
> Thiago



-- 
Thiago

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by "Thiago H. de Paula Figueiredo" <th...@gmail.com>.
On Tue, Jun 7, 2022 at 8:31 AM Volker Lamp <vl...@apache.org> wrote:

> TAP5-2718 (Tapestry Announcement lacks version) is still showing 'UNKNOWN'. I reopened the issue and pushed a test case to a new branch.

Thanks for testing it! Yeah, it wasn't fixed properly, but now it is.

> TAP5-2721 Broken links to 5.8.1 asc files on download page
> If it's a "can't fix" the ticket should be decorated :) with a comment and then closed, so that the reporter receives feedback.

I just did exactly that. :)

>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>


-- 
Thiago

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by Volker Lamp <vl...@apache.org>.
> > > TAP5-2714 Checkbox component with Bootstrap 4
> > Something similar and pretty simple was done in GridPager: @Inject
> > org.apache.tapestry5.services.compatibility.Compatibility and call
> > compatibility.enabled(Trait.BOOTSTRAP_4) to check whether it's running
> > with Bootstrap 4 or not, then output the class attribute accordingly.
> 
> I went ahead and implemented my suggestion above. Have we cleared out
> our to-do list?

TAP5-2718 (Tapestry Announcement lacks version) is still showing 'UNKNOWN'. I reopened the issue and pushed a test case to a new branch.

TAP5-2721 Broken links to 5.8.1 asc files on download page
If it's a "can't fix" the ticket should be decorated :) with a comment and then closed, so that the reporter receives feedback.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by "Thiago H. de Paula Figueiredo" <th...@gmail.com>.
I believe you can push your local branch to Gitbox without issues
regardless of whether it's been pushed to GitHub before. I closed a
few GitHub pull requests by checking out from GitHub then
cherry-picking the changes from the local branch into master.

On Tue, Jun 7, 2022 at 8:45 AM Ben Weidig <be...@netzgut.net> wrote:
>
> Hi Volker,
>
> I've switched to GitBox because it's the canonical remote and not just the
> mirrored one.
> It seems that Apache had some infrastructure issues lately affecting the
> mirroring, which caused the remotes to differ.
> My train of thought is that I don't want to merge back a branch to master
> that isn't even present on GitBox, because I'm not sure how GitBox would
> react.
>
> But maybe the easiest way forward is creating a diff, deleting the branch
> on GitHub, and committing/pushing the diff on GitBox.
>
>
> On Tue, Jun 7, 2022 at 1:38 PM Volker Lamp <vl...@apache.org> wrote:
>
> > Hi Ben
> >
> > > Any suggestions for the best way to solve this?
> > > So far, I've worked with GitHub, but now I've switched my repo's remote
> > > origin to GitBox.
> > > Should I just push the branch to GitBox and hope for the best that
> > > mirroring works and nothing clashes with the pre-existing branch?
> > > Delete the GitHub branch first?
> >
> > How about setting origin back to GitHub? That way you'd at least push to a
> > repo that already has the branch.
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> > For additional commands, e-mail: dev-help@tapestry.apache.org
> >
> >



-- 
Thiago

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by Ben Weidig <be...@netzgut.net>.
Hi Volker,

I've switched to GitBox because it's the canonical remote and not just the
mirrored one.
It seems that Apache had some infrastructure issues lately affecting the
mirroring, which caused the remotes to differ.
My train of thought is that I don't want to merge back a branch to master
that isn't even present on GitBox, because I'm not sure how GitBox would
react.

But maybe the easiest way forward is creating a diff, deleting the branch
on GitHub, and committing/pushing the diff on GitBox.


On Tue, Jun 7, 2022 at 1:38 PM Volker Lamp <vl...@apache.org> wrote:

> Hi Ben
>
> > Any suggestions for the best way to solve this?
> > So far, I've worked with GitHub, but now I've switched my repo's remote
> > origin to GitBox.
> > Should I just push the branch to GitBox and hope for the best that
> > mirroring works and nothing clashes with the pre-existing branch?
> > Delete the GitHub branch first?
>
> How about setting origin back to GitHub? That way you'd at least push to a
> repo that already has the branch.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
>

Re: Thoughts for 5.8.2

Posted by Volker Lamp <vl...@apache.org>.
Hi Ben

> Any suggestions for the best way to solve this?
> So far, I've worked with GitHub, but now I've switched my repo's remote
> origin to GitBox.
> Should I just push the branch to GitBox and hope for the best that
> mirroring works and nothing clashes with the pre-existing branch?
> Delete the GitHub branch first?

How about setting origin back to GitHub? That way you'd at least push to a repo that already has the branch.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by Ben Weidig <be...@netzgut.net>.
I would like to merge my TAP5-2728 branch with the EventContext
enhancements, but it's not there...

GitHub has it and Travis CI build it (but failed due to other tests).

https://github.com/apache/tapestry-5/tree/TAP5-2728
https://app.travis-ci.com/github/apache/tapestry-5/builds/251361528

But it seems not be on GitBox, though

https://gitbox.apache.org/repos/asf?p=tapestry-5.git

Any suggestions for the best way to solve this?
So far, I've worked with GitHub, but now I've switched my repo's remote
origin to GitBox.
Should I just push the branch to GitBox and hope for the best that
mirroring works and nothing clashes with the pre-existing branch?
Delete the GitHub branch first?

Cheers!
Ben

On Tue, Jun 7, 2022 at 5:22 AM Thiago H. de Paula Figueiredo <
thiagohp@gmail.com> wrote:

> On Tue, May 31, 2022 at 10:18 PM Thiago H. de Paula Figueiredo
> <th...@gmail.com> wrote:
> >
> > On Tue, May 31, 2022 at 5:57 PM Volker Lamp <vl...@apache.org> wrote:
> > > TAP5-2714 Checkbox component with Bootstrap 4
> >
> > Something similar and pretty simple was done in GridPager: @Inject
> > org.apache.tapestry5.services.compatibility.Compatibility and call
> > compatibility.enabled(Trait.BOOTSTRAP_4) to check whether it's running
> > with Bootstrap 4 or not, then output the class attribute accordingly.
>
> I went ahead and implemented my suggestion above. Have we cleared out
> our to-do list?
>
> Cheers!
>
> --
> Thiago
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>
>

Re: Thoughts for 5.8.2

Posted by "Thiago H. de Paula Figueiredo" <th...@gmail.com>.
On Tue, May 31, 2022 at 10:18 PM Thiago H. de Paula Figueiredo
<th...@gmail.com> wrote:
>
> On Tue, May 31, 2022 at 5:57 PM Volker Lamp <vl...@apache.org> wrote:
> > TAP5-2714 Checkbox component with Bootstrap 4
>
> Something similar and pretty simple was done in GridPager: @Inject
> org.apache.tapestry5.services.compatibility.Compatibility and call
> compatibility.enabled(Trait.BOOTSTRAP_4) to check whether it's running
> with Bootstrap 4 or not, then output the class attribute accordingly.

I went ahead and implemented my suggestion above. Have we cleared out
our to-do list?

Cheers!

-- 
Thiago

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by "Thiago H. de Paula Figueiredo" <th...@gmail.com>.
On Wed, Jun 1, 2022 at 3:51 AM Ben Weidig <be...@netzgut.net> wrote:
> Hello!

Hello!

> That's a point on my (neverending) todo-list: going through the most recent
> tickets and check if they're still relevant and update accordingly, so we
> have a clearer picture of the actually open issues.

That's a great idea. In addition, I guess we have at least some old
tickets describing issues which are no longer a bug.

> I'm thinking about switching to signed commits,

For Tapestry or somewhere else? :)

> I got one addition to 5.8.2 that's not on the list so far, a ticket I've
> just created: TAP5-2728 EventContext enhancements.
>
> It adds two new methods to EventContext:
>
>    - boolean isEmpty()
>    - <T> Optional<T> tryGet(Class<T> desiredType, int index)

I really like this idea.

Cheers!

--
Thiago

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org


Re: Thoughts for 5.8.2

Posted by Ben Weidig <be...@netzgut.net>.
Hello!


> It seems to be fixed and has a very nice unit test. I've closed it. By
> the way, the other day I was looking at tickets and there were a
> number of them with fixes but apparently people forgot to update the
> ticket accordingly.
>

That's a point on my (neverending) todo-list: going through the most recent
tickets and check if they're still relevant and update accordingly, so we
have a clearer picture of the actually open issues.


We don't need to wait a few months to cut a new release. This has
> happened due to lack of stuff to include in a release. Any committer
> of the Tapestry team can do a release. It requires some effort,
> specially the first time (setting up GnuPG, etc), and the process is
> documented in the Tapestry site.
>

I'm thinking about switching to signed commits, so I need to get used to
GnuPG anyway.
Going to check out the release guide if I find some time.



> Another possibility is to check the original source: the first commit,
> 11 years ago (!!!) says the code is borrowed from Quartz. Our copy
> hasn't been updated ever since, while I'd guess Quartz's one has been
> changed a number of times since then:
>
> https://github.com/quartz-scheduler/quartz/blob/master/quartz-core/src/main/java/org/quartz/CronExpression.java
> .
>

I concur with Thiago that it shouldn't postpone the release.
Hopefully, the code just have to be updated with any changes since it was
introduced.
The test case will help, thanks!


I got one addition to 5.8.2 that's not on the list so far, a ticket I've
just created: TAP5-2728 EventContext enhancements.

It adds two new methods to EventContext:

   - boolean isEmpty()
   - <T> Optional<T> tryGet(Class<T> desiredType, int index)


The change includes unit tests, but I switched the existing ones to
tapestry-spock, so I've added the project as a test dependency.

This time, I'll create a new branch, so you can check out the code before
it hopefully ends up in master.


Cheers
Ben

Re: Thoughts for 5.8.2

Posted by "Thiago H. de Paula Figueiredo" <th...@gmail.com>.
On Tue, May 31, 2022 at 5:57 PM Volker Lamp <vl...@apache.org> wrote:

> Hi guys

Hello! And thanks for creating this new thread and discussion! I was
very, very right when I said Volker would be a great addition to the
team and it's showing already. :)

> Pickung up what Thiago's wrote in another thread recently -
>
> > What about a release next week? I've just committed CORS support and
> > I'm looking forward to get it released. :)
>
> Here are my thoughts about the next release. Well done for adding CORS
> support, Thiago. It's a useful addition to Tapestry.

Thanks! Nothing complicated or fancy, but I guess it still can be useful. :)

> Further, we have a
> number of smaller bug fixes and improvements. I'm all for releasing
> early, however, I think we should make an effort towards including the
> fixes for the issues listed below also:
>
> TAP5-2718 Tapestry Announcement lacks version

I believe I've found the problem and I've just committed a fix.

> TAP5-2714 Checkbox component with Bootstrap 4

Something similar and pretty simple was done in GridPager: @Inject
org.apache.tapestry5.services.compatibility.Compatibility and call
compatibility.enabled(Trait.BOOTSTRAP_4) to check whether it's running
with Bootstrap 4 or not, then output the class attribute accordingly.

> TAP5-2721 Broken links to 5.8.1 asc files on download
>            page (more a todo than a commit)

I believe this will be a won't fix. It seems the .asc file generation
got broken or missing during the Gradle upgrade. I'm not sure these
files are actually needed according to ASF policies. If not, there's
already 2 other checksums and we can just ignore this from now on.

> The above are probably not very complicated or challenging. Fixing them
> can make a difference for new users, though  - no second chance for a
> first impression.

Agreed.

> TAP5-2713 ChecksumPath
> I understand Ben already fixed this one, so the issue could be set to
> closed also.

It seems to be fixed and has a very nice unit test. I've closed it. By
the way, the other day I was looking at tickets and there were a
number of them with fixes but apparently people forgot to update the
ticket accordingly.

> TAP5-2723 CronExpression fails handling L-<dayOfMonth>
> It would be nice if Charles didn't have to wait another few months for a
> fix.

We don't need to wait a few months to cut a new release. This has
happened due to lack of stuff to include in a release. Any committer
of the Tapestry team can do a release. It requires some effort,
specially the first time (setting up GnuPG, etc), and the process is
documented in the Tapestry site.

> That said, I agree with Ben: the code in CronExpression isn't easy
> to understand and hence the cause of the problem not easily spotted.

That's a huge understatement. :) I'd never looked at this class and,
whoa, that's complicated.

> My small contribution so far is a test case which I added to the TAP5-2723
> branch.

This is not a small contribution. It's a big one.

>  I won't have the time to work on it more during the next couple
> of weeks, so either somebody else picks it up or we just leave it until
> later.

I won't have enough time for that in the next few weeks either, so,
unless somebody else can pick it up, I vote for leaving it to later.

Another possibility is to check the original source: the first commit,
11 years ago (!!!) says the code is borrowed from Quartz. Our copy
hasn't been updated ever since, while I'd guess Quartz's one has been
changed a number of times since then:
https://github.com/quartz-scheduler/quartz/blob/master/quartz-core/src/main/java/org/quartz/CronExpression.java.

> Thanks,

Thank *you*!

>
> Volker
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
> For additional commands, e-mail: dev-help@tapestry.apache.org
>


-- 
Thiago

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tapestry.apache.org
For additional commands, e-mail: dev-help@tapestry.apache.org