Closed hazendaz closed 1 year ago
Feel free to squash if necessary. My style on commits tends to follow a convention allowing for easy walk of history without having to look at the content so much as well as easier way to revert if one commit causes issues. As I use github desktop, its single line applications sometimes order funny which was case on a couple here when individually looked at. Overall just improved this.
I now see why Gh-actions was removed but that doesn't help the community. The community doesn't have your Jenkins usage. Both should be otherwise fine and in fact, could just rework what I did slightly to tell it to not run on your repo so that it runs on contributors only. I can assist in doing that if needed. The important thing here is to show this works against as many types as possible and further allow contributors to know it actually works on multiple platforms as otherwise cannot tell.
@skuzzle I'm unable to see the issues on jenkins. Also given I'm adding back GH-Actions here, it requires you to approve to run, you can see mine here I believe https://github.com/hazendaz/restrict-imports-enforcer-rule/actions/runs/4774455713.
one other thing, I did read your contributing doc, while I didn't open issue here, this was no time at all. with most OSS, I know its ill advised to bring so many changes at once but most of this was pretty straight forwards and I'm willing to break up into any parts needed. Additionally, can add more github actions to do such things as run on older maven versions to confirm how far back this can actually go. Gut would tell me based on usage here at least back to 3.3.9 but probably even further. Main thing is fact that maven parts are set to provided and no code changes needed to occur to use latest. For our usage, we are using both maven 3.8.x and 3.9.x with this currently at 2.1.0 and many prior versions.
Hey. I only briefly checked your changes but I like them so far. Here are 2 first notes:
I also like the idea of testing against multiple maven versions. If it is no big deal feel free to add it as well or in a separate PR
- I've just recently added the spotless plugin and I've seen that it supports pom formatting/sorting as well. Might be a sweet addition to configure it here to enforce a certain layout
There is a plugin called sortpom-maven-plugin that specifically follows maven accepted layout. For pom.xml, its probably best choice given it entirely knows about maven pom standards. Spotless while setup here is mostly just treating it as sticking with spaces, spaces per tab, and trim trailing whitespace, all good things but sortpom-maven-plugin will go a step further and enforce the layout. Certainly could add that into this.
- PR must be filed against the develop branch. Changes will be merged to master when I do a release
Switched to target the develop branch.
I also like the idea of testing against multiple maven versions. If it is no big deal feel free to add it as well or in a separate PR
Added maven 3.3.9 and 3.9.1 as runtime usage, running in verify now so integration tests pass. Looking at spotless as it fails jdk 21, may just add skip profile for that as probably overkill to run spotless on every jdk until they support 21.
Guess we could look at this spotless
sortPom
[homepage](https://github.com/Ekryd/sortpom). [code](https://github.com/diffplug/spotless/tree/main/plugin-maven/src/main/java/com/diffplug/spotless/maven/pom/SortPom.java).
All configuration settings are optional, they are described in detail [here](https://github.com/Ekryd/sortpom/wiki/Parameters).
Ok. After getting integration tests running, having few issues with windows platform on github. Cleared both Linux and Mac. Got maven 3.3.9 and 3.9.1 running. Will spend some additional cycles in coming days and will update PR with addition commits as ready.
Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Simon Taddiken @.> Sent: Monday, April 24, 2023 2:03:18 AM To: skuzzle/restrict-imports-enforcer-rule @.> Cc: Jeremy Landis @.>; Author @.> Subject: Re: [skuzzle/restrict-imports-enforcer-rule] Re-board GH-Actions and updated all underlying libraries (PR #62)
@skuzzle commented on this pull request.
In pom.xmlhttps://github.com/skuzzle/restrict-imports-enforcer-rule/pull/62#discussion_r1174821257:
<version.invoker-plugin>3.5.1</version.invoker-plugin>
2.34.0 2.36.0
I see why it doesn't build on my jenkins right now. This spotless plugin version currently doesn't work on my jenkins because it tries to access a file at a location where it has no permissions. I haven't had time to properly fix this so for now we need to stick with the slightly older version.
— Reply to this email directly, view it on GitHubhttps://github.com/skuzzle/restrict-imports-enforcer-rule/pull/62#pullrequestreview-1397245265, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAHODIZP6W34NEOBU7DH4JTXCYJSNANCNFSM6AAAAAAXIA6REI. You are receiving this because you authored the thread.Message ID: @.***>
Are you still working on this? I'm really keen on getting this in for the various improvements you made. We could also simplify the GH actions build for now if it is too hard to make it work with the whole matrix of parameters
So sorry totally lost track on this. Will look this weekend to see where I was. Thanks for reminder.
Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Simon Taddiken @.> Sent: Wednesday, May 10, 2023 3:32:35 AM To: skuzzle/restrict-imports-enforcer-rule @.> Cc: Jeremy Landis @.>; Author @.> Subject: Re: [skuzzle/restrict-imports-enforcer-rule] Re-board GH-Actions and updated all underlying libraries (PR #62)
Are you still working on this? I'm really keen on getting this in for the various improvements you made. We could also simplify the GH actions build for now if it is too hard to make it work with the whole matrix of parameters
— Reply to this email directly, view it on GitHubhttps://github.com/skuzzle/restrict-imports-enforcer-rule/pull/62#issuecomment-1541495506, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAHODI2KJEJEODUSQ4JES5LXFNABHANCNFSM6AAAAAAXIA6REI. You are receiving this because you authored the thread.Message ID: @.***>
As development here stalled a bit I manually applied the dependency updates to the develop
branch. Feel free to work on the GH-Actions thing as you see fit.
Sorry I've been limited on time recently. Trying to dig out of maven warning alerts on many plugins leaving me with little time since maven 3.9.2. This is still on my radar otherwise. Thanks.
Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Simon Taddiken @.> Sent: Friday, May 26, 2023 6:37:56 AM To: skuzzle/restrict-imports-enforcer-rule @.> Cc: Jeremy Landis @.>; Author @.> Subject: Re: [skuzzle/restrict-imports-enforcer-rule] Re-board GH-Actions and updated all underlying libraries (PR #62)
As development here stalled a bit I manually applied the dependency updates to the develop branch. Feel free to work on the GH-Actions thing as you see fit.
— Reply to this email directly, view it on GitHubhttps://github.com/skuzzle/restrict-imports-enforcer-rule/pull/62#issuecomment-1564187335, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAHODI6ZEEDSGRT6QQ3FSITXICBYJANCNFSM6AAAAAAXIA6REI. You are receiving this because you authored the thread.Message ID: @.***>
In preparation for #59 this project will soon build with gradle (see #105). This makes this PR obsolete. On a related note, my jenkins now publishes more details to the Github checks in case a build failed. Thus it is not necessary to also build with GitHub actions. Example: