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

Add check_remote_head option to avoid unnecessary new releases by che… #1759

Merged
merged 11 commits into from Aug 6, 2019
Merged

Add check_remote_head option to avoid unnecessary new releases by che… #1759

merged 11 commits into from Aug 6, 2019

Conversation

ahmadmayahi
Copy link
Contributor

Checking the remote git HEAD without cloning the repo.

Q A
Bug fix? No
New feature? Yes
BC breaks? No
Deprecations? No
Fixed tickets #1755

A new option check_remote_head has been added, when this option sets to true deployer will not clone the repository if there are no changes found.

Thanks to git ls-remote, this command gets the HEAD without cloning the repository.

Revision is also checked, so if we run something like dep deploy --revision=a69539703d917cafb1c51dad20f4ee1e317c3017 deployer will also check for the revision.

@antonmedv antonmedv self-requested a review November 8, 2018 01:40
@antonmedv antonmedv self-assigned this Nov 8, 2018
@antonmedv
Copy link
Member

Build failed

@ahmadmayahi
Copy link
Contributor Author

Build failed

It keeps saying Deployer\Support\Changelog\ParseException: Error in compare link syntax I have no idea why?

@antonmedv
Copy link
Member

Try checkout CHANGELOG and run php bin/changelog

@ahmadmayahi
Copy link
Contributor Author

I tried that, but still get the same result:
Image of Yaktocat

@antonmedv
Copy link
Member

Really strange, will look at this tonight.

@antonmedv antonmedv assigned ahmadmayahi and unassigned antonmedv Nov 8, 2018
@antonmedv
Copy link
Member

@ahmadmayahi please, start with this PR. Try to figure out why CI doesn't passing. Maybe bug in changelog's parser.

Removed extra spaces: #1759
@cafferata
Copy link
Contributor

@antonmedv, this is because @ahmadmayahi added spaces after the following line

[v6.3.0...master](https://github.com/deployphp/deployer/compare/v6.3.0...master)

I submitted an pull request to the ahmadmayahi/deployer fork: https://github.com/ahmadmayahi/deployer/pull/1

@ahmadmayahi
Copy link
Contributor Author

Thank you @cafferata

recipe/deploy/prepare.php Outdated Show resolved Hide resolved
recipe/deploy/prepare.php Outdated Show resolved Hide resolved
get('bin/git'), $repository)));
if (true === get('check_remote_head') && null == input()->getOption('tag')) {
$headPath = trim(get('deploy_path').'/.dep/HEAD');
$isRemoteHeadExists = test("[ -f $headPath ]");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this check really required, or could it be achieved in combination with the „cat“ line in a single command?
Every remote command has a big overhead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, cating non-existing file leads to STDERR.
I know, we could output the error to the 2>/dev/null but I think it's a hack, the right solution is to check whether the file exists or not.

Copy link
Contributor

@staabm staabm Nov 12, 2018

Choose a reason for hiding this comment

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

And something along the lines of
run('test -f '. $headPath ' && cat '.$headPath);

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How could test become a part of the run command? I guess you mean the output of test right?

Copy link
Member

Choose a reason for hiding this comment

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

@staabm
Copy link
Contributor

staabm commented Aug 5, 2019

ping @antonmedv

@antonmedv antonmedv merged commit bb4d41c into deployphp:master Aug 6, 2019
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.

None yet

4 participants