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

branch.gitpublishto overrides explicit profile to option #23

Open
ehabkost opened this issue Mar 31, 2017 · 3 comments
Open

branch.gitpublishto overrides explicit profile to option #23

ehabkost opened this issue Mar 31, 2017 · 3 comments

Comments

@ehabkost
Copy link
Contributor

ehabkost commented Mar 31, 2017

If the user has the following on .gitpublish:

[gitpublishprofile "SOMEPROFILE"]
to = SOMEBODY

and they run:

$ git-publish --profile SOMEPROFILE --to SOMEBODYELSE

and then run:

$ git-publish --profile SOMEPROFILE

I believe the intended recipient for the message is SOMEBODY, not SOMEBODYELSE, as 'to' is explicitly configured in the .gitpublish profile. But git-publish will send the message to SOMEBODYELSE instead of SOMEBODY.

Is this a bug or a feature?

Considering that a profile was explicitly chosen in the command-line using --profile, I believe the profile options should take precedence over any other options from other config files.

@stefanha
Copy link
Owner

stefanha commented Mar 31, 2017 via email

@ehabkost
Copy link
Contributor Author

Do you want to send a patch?

I would like to, but I will probably take a while before I reserve time for that. It looks like there are many variables that would require the same rule to be implemented, not just to.

@jnsnow
Copy link
Contributor

jnsnow commented Sep 5, 2019

Is this right, actually?

If you have manually set an explicit --to and it gets saved to the branch configuration, isn't it semantically the case that the branch has more local and specific configuration than the .gitpublish file which is intended to be a repo-packaged file?

I guess the problem might be for branches that see frequent re-use. If you have a topic branch like "mycoolfeature" and you set an explicit --to (intentionally overriding the .gitpublish profile), when you run that command later it's actually very likely you still want the saved values and not to revert to the defaults.

At least, that's how I understood git-publish to work: when you omit values, it tries to use the values you used for the last invocation.

In what case would the saved values be incorrect? The problem might be for frequently re-used branches where you actually aren't intending to load a saved configuration, but instead sending a new unrelated pull request or are starting a new series from v1.

Or, perhaps, the problem is that --profile foo is interpreted by the user as a request to load those explicit values instead of loading those as default values.

Maybe this could be handled as such:

  • Store the last used profile in the branch config. When re-issuing git-publish, load that profile.
  • If a profile is explicitly set that doesn't match the last-used one, we have semantic knowledge that the user is intentionally changing profiles and is likely not interested in the saved configuration.
  • On switch, existing branch configuration can be dropped entirely (?) or
  • On switch, do a one-time re-write of the profile variables over the branch variables, leaving additional settings not in-conflict intact.

Or perhaps the problem is just that we sometimes want to drop 'remembered' values to get a clean slate when a previous version had a mistake, and we want a --forget option that covers all of the other similar options that drop previous CC lists.

Though, because the branch config is used both by the user for explicit configuration and by git-publish as its "memory" for saving you typing on re-invocation, it's not clear which values we want to delete, actually.

Do we actually want separate branch config sections versus a separate "memory" section?
(e.g. branch.{topic}.{profile}.memory gitpublishcc, gitpublishto, etc ...) This way it would always be clear which values the user is intending to forget and which represent intentional manual config.

(Wait, is git smart enough to delete config that's sub-spaced from branch.{topic} on branch delete?)

Of course, since config values would now span a dizzying array of possible locations, the configuration fetchers would need to be refactored and centralized a bit to canonize a lookup order instead of open-coding it for each argument.

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

No branches or pull requests

3 participants