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.

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

Remove Secondary POM file #4278

Merged
merged 3 commits into from
Aug 21, 2020
Merged

Remove Secondary POM file #4278

merged 3 commits into from
Aug 21, 2020

Conversation

TheFlagCourier
Copy link
Member

Description:

Removes the long-standing "temporary" pom_deb.xml file used by GitHub Actions and some linux users, replaced with a maven profile.


New Nodes/Commands/ConfigOptions:

mvn [command] -P github-action will now use the same setup as pom_deb.xml had for Javadocs.


Relevant Towny Issue ticket:

N/A - But does fix Actions' Publish Release action - which stalled on org.apache.commons.compress - since it wasn't in pom_deb.xml


  • I have tested this pull request for defects on a server. Not Applicable.
  • I have tested for build failures. The appropriate JARs are built.

By making this pull request, I represent that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the TownyAdvanced organization has the copyright to use and modify my contribution under the Towny License for perpetuity.

Adds a profile to the main pom for when it's actually
necessary.

Current implementation has several pitfalls, such as
outdated configuration, and was prone to human error.

Fixes a Deployment action, which fails due to
org.apache.commons.compress missing in the pom_deb.xml
Actually remove it.
@LlmDl
Copy link
Member

LlmDl commented Aug 20, 2020

nice

@ghost
Copy link

ghost commented Aug 21, 2020

I don't know if it's intended or not, but the GitHub actions compiling using -P -github-action doesn't generate any javadocs which can be seen in the log. The <executions> group is missing in the javadoc plugin so it's not doing anything.

Also, I'm not sure we need a deploy-on-release action anymore, mainly because we don't do any new releases via GitHub packages anymore, which our pom.xml deploys to. Jitpack automatically picks up new releases compiles them and releases them (as well as generates javadocs).

@TheFlagCourier
Copy link
Member Author

I don't know if it's intended or not, but the GitHub actions compiling using -P -github-action doesn't generate any javadocs which can be seen in the log. The <executions> group is missing in the javadoc plugin so it's not doing anything.

Also, I'm not sure we need a deploy-on-release action anymore, mainly because we don't do any new releases via GitHub packages anymore, which our pom.xml deploys to. Jitpack automatically picks up new releases compiles them and releases them (as well as generates javadocs).

Valid points. While the compile wasn't necessary, it was added in the event anything is specifically done to it in the future. The profile is called github-action, not javadoc-fix.

As for the deploy, those actions are still presently executed. (And failing due to the error mentioned in the PR.) Even with removing deploy actions, the profile is still beneficial to certain OS environments like Debian.

@ghost
Copy link

ghost commented Aug 21, 2020

As for the deploy, those actions are still presently executed. (And failing due to the error mentioned in the PR.) Even with removing deploy actions, the profile is still beneficial to certain OS environments like Debian.

I realize that these were failing but, regardless that doesn't change the fact that we don't deploy on GitHub packages anymore. Hence even while it was failing it didn't affect us, because we don't use it. The reason we originally made a separate pom for debian-based distributions was because the javadoc plugin was failing to find it's executable, this has since been fixed. This issue occurs when the $JAVA_HOME path var isn't properly set which can be fixed by running:

export JAVA_HOME=$(update-alternatives --query javadoc | grep Value: | head -n1 | sed 's/Value: //' | sed 's@bin/javadoc$@@')

(Tested on Debian 10 Buster)

But as it stands now we only use the javadoc plugin for when it's uploaded to Jitpack (which also runs an Ubuntu VM to compile jar and generate javadocs) and there has been no issue on there as well.

@TheFlagCourier
Copy link
Member Author

I understand where you're coming from, and that it is a configuration error on some systems. However, not everyone is going to jump through the hoops to fix their path.

Also, it seems the intent is to push to both JitPack and Github simultaneously.
image

@LlmDl LlmDl merged commit 452b17f into master Aug 21, 2020
@LlmDl LlmDl deleted the housekeeping/remove-secondary-pom branch August 21, 2020 15:44
@LlmDl LlmDl added this to the 0.96.3.0 milestone Aug 21, 2020
LlmDl added a commit that referenced this pull request Aug 21, 2020
    - Removes towns.txt and nations.txt files on flatfile databases.
      - Loads old towns.txt and nations.txt to ensure that orphaned
files in the
        towns & nations folders are not loaded. If successfully loaded,
the
        towns.txt and nations.txt files are deleted.
    - Residents now store their Town, and Towns do not store their
Residents.
    - Towns now store their Nation, and Nations do not store their
Towns.
    - Towny now handles loading a mayor for a town intelligently.
      - If a town attempts to set a mayor who is not part of the town,
and the
        town has no residents, the town will be deleted.
      - Closes #4170.
    - Towny now handles loading the capital of a nation intelligently.
  - Little fixes for TownySQLSource:
    - Hit TownySQLSource with the formatting stick.
    - Add tar/tar.gz backups to the backup code.
    - Make deletePlotGroup actually do something.
  - Invite Cleanup, courtesy of Siris with PR #4269.
  - Cleanup Confirmation class and properly implement async, courtesy of
Siris with PR #4280.
  - Remove Secondary POM file, courtesy of TheFlagCourier with PR #4278.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants