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.

Sign up for GitHub

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

Jump to bottom

Add support for defining version tags instead of branches #372

Merged
merged 6 commits into from
Nov 27, 2020
Merged

Add support for defining version tags instead of branches #372

merged 6 commits into from
Nov 27, 2020

Conversation

SenseException
Copy link
Member

Handles #369

@SenseException SenseException linked an issue Nov 1, 2020 that may be closed by this pull request
Add support for defining version tags instead of branches #369
Closed
@SenseException SenseException mentioned this pull request Nov 1, 2020
Merged
@SenseException
Copy link
Member Author

I started to test a PoC with a temporary repository yesterday. If this is working, I'll finalize this draft and make a PR out of it.

@SenseException SenseException force-pushed the docs-of-tags branch 2 times, most recently from 9d1aa7c to 0f98b10 Compare November 21, 2020 01:14
@SenseException
Improve readability of ProjectVersionReaderTest
c076b45 
Make test more readable and removes implicit order of calls. This should
also improve the documentation purpose of the test and its expected
result.
@SenseException
Add test for branchslug skip
1846d54
@SenseException
Add support to checkout tags
acd8b97
@SenseException
Allow nullable branchnames
6d10b76
@SenseException
Set branchName to null if missing in project data
40949b5 
Usually the branchName values are equal between $version and $projectVersion
exept when branchName was omitted in the project's website config which
results in a null-value.
@SenseException SenseException marked this pull request as ready for review November 22, 2020 23:01
@SenseException
Copy link
Member Author

SenseException commented Nov 22, 2020 •
edited
Loading

If no branchName is given or set to null, the latest tag will be used for the build of the docs. @morozov

morozov
morozov reviewed Nov 23, 2020
View reviewed changes
Copy link
Member

@morozov morozov left a comment

Choose a reason for hiding this comment

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

Thank you, @SenseException! Looking forward to being able to use this.

lib/Projects/ProjectGitSyncer.php Outdated
$command = sprintf(
'cd %s && git clean -xdf && git checkout tags/%s',
$this->getRepositoryPath($repositoryName),
$tagName
Copy link
Member

Choose a reason for hiding this comment

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

Although it will be used only for doctrine-owned projects, it still makes sense to implement proper escaping of the command line arguments.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Done.

greg0ire
greg0ire reviewed Nov 24, 2020
View reviewed changes
Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

This was mentioned internally in Slack and using tags instead of branches is going to be a new additional configuration in .doctrine-project.json files.

So how does it work? Should we document that somewhere?

@SenseException
Copy link
Member Author

Omit branchName or set it to null. Then the newest tagname will be used ("newest" as the used method name suggested and manual tests showed).

Should we document that somewhere?

The documentation has no documentation 🤔
I should start to document .doctrine-project.json. At least as much as I know about it. I would like to create a separate PR for that.

greg0ire
greg0ire approved these changes Nov 25, 2020
View reviewed changes
Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Looks OK as is.

@SenseException SenseException merged commit 6076042 into master Nov 27, 2020
@SenseException SenseException deleted the docs-of-tags branch November 27, 2020 18:55
@morozov morozov mentioned this pull request Nov 28, 2020
Merged
@SenseException SenseException mentioned this pull request Jan 20, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@morozov morozov morozov left review comments

@greg0ire greg0ire greg0ire approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for defining version tags instead of branches
3 participants
@SenseException @morozov @greg0ire