Skip to content
This repository was archived by the owner on May 10, 2023. It is now read-only.

Conversation

@afragen
Copy link
Collaborator

@afragen afragen commented Dec 23, 2019

Now uses /.wordpress-org as location for dot org assets. Since 10up has created the GitHub Action for similar purposes, it seems this naming structure will be better.
Fixes https://github.com/GaryJones/wordpress-plugin-svn-deploy/issues/31

Since 10up has created the GitHub Action for similar purposes, it seems this naming structure will be better.
Fixes https://github.com/GaryJones/wordpress-plugin-svn-deploy/issues/31
fix typo
Copy link
Owner

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The readme would also need updating.

Also, how about asking the user what the directory name is of their asserts directory, perhaps defaulting to .wordpress-org? (If so, another step in the readme would be needed).

@afragen
Copy link
Collaborator Author

afragen commented Dec 23, 2019

I just updated the README. I also bumped the version as this would be a breaking change.

I think we should commit to a standard, with respect to the location of the dot org assets.

I don't believe we need to provide an option for the user to name their own repo folder containing objects that would need updating to SVN /assets. Decsions, not options. Besides this would allow users more flexibility to switch to the GitHub Actions or back without needing to further modify their repo.

@afragen afragen requested a review from GaryJones December 24, 2019 01:11
@GaryJones
Copy link
Owner

I think we should commit to a standard, with respect to the location of the dot org assets.

But, there is no standard.

standards

The decision we make, is setting the default value. For any new users, that shows the intent, which I'm happy to follow. For any existing users who update to the major version, then being prompted allows them a quick way of still getting their deployment done without having to stop and start renaming their directory.

@afragen
Copy link
Collaborator Author

afragen commented Dec 24, 2019

I didn't mean to imply that .wordpress-org was a standard. I only meant to imply that it's now being touted by @10up with their GitHub Action and it's seems as good as anything.

That and we have at least one issue with a user wanting to have an assets dir in their plugin. #31

It seems much more reasonable to default to something that isn't likely to be used for anything else, but I can certainly add a setting ask with .wordpress-org as default. I don't really think we/I took very much consideration into using the exact same name as on SVN. 🤦‍♂

@afragen
Copy link
Collaborator Author

afragen commented Dec 24, 2019

@GaryJones Any thoughts as to which step number this should be?

I'm thinking it's a new step 3

@GaryJones
Copy link
Owner

I'd say a new Q3 (after the current step 3, which is checking that the current Q2 is valid).

@afragen
Copy link
Collaborator Author

afragen commented Dec 24, 2019

How does this look? Seems to test out fine in a quick run without actual svn commit

@afragen
Copy link
Collaborator Author

afragen commented Dec 24, 2019

I took out the linting updates as it distorted what we want to accomplish.

@GaryJones
Copy link
Owner

Looks like we need a 3.1.0 tag before you tag a 4.0.0 ;-)

@afragen
Copy link
Collaborator Author

afragen commented Dec 26, 2019

So a 3.1.0 tag off the current develop branch?

I haven’t really tagged releases here just sort of bumped the occasional version number. 😉

Happy Christmas 🎄

@GaryJones
Copy link
Owner

Well, on those commits where you bumped the version, tag (oldest first) and push those tags, before doing 4.0 :-)

@afragen afragen merged commit 049770d into GaryJones:develop Dec 26, 2019
@afragen afragen deleted the wp-org-assets branch December 26, 2019 17:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handle presence of assets folder in plugin

2 participants