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

Making node id optional for all issue commands (except branch) #56

Merged
merged 2 commits into from
May 4, 2020

Conversation

hansfn
Copy link
Contributor

@hansfn hansfn commented May 2, 2020

Partially fixes #53.

If I get time, I'll do the same for project name.

Also refactoring to avoid some duplicated code.
@mglaman mglaman force-pushed the node-id-optional branch from 628b18c to f686d2c Compare May 3, 2020 01:24
@mglaman
Copy link
Owner

mglaman commented May 3, 2020

Rebased against master to fix conflicts. Making a few changes. I think it's fine to always have IssueCommandBase run initRepo instead of conditionally running it again in each command. Cleans things up a little.

@mglaman
Copy link
Owner

mglaman commented May 3, 2020

@hansfn want to give my changes a spin? Then we can merge and follow up with improving the Git repository detection

@hansfn
Copy link
Contributor Author

hansfn commented May 3, 2020

The changes are fine - just merge them. (I see I forgot to use initRepo in Apply::initialize.)

The reason I kept initRepo in the specific command subclasses was to be able to keep one feature of the link command - it worked without a local repo. I don't mind loosing that feature. (I have defined search engines in Chrome with keyword "don" - drupal.ord/node/%s - so I just type "don NID" in the address, anyway.)

@mglaman
Copy link
Owner

mglaman commented May 4, 2020

Ah, yeah I didn't think about that for link. I'll just merge and open a follow up issue.

@mglaman mglaman merged commit 306405a into mglaman:master May 4, 2020
@hansfn hansfn deleted the node-id-optional branch July 20, 2021 12:29
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.

UX: Making node ID and project name (ID) optional
2 participants