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

Added declarative pipeline example #412

Conversation

Jagrutiti
Copy link
Contributor

@Jagrutiti Jagrutiti commented Nov 23, 2022

Close #322

Testing done

NN

Proposed upgrade guidelines

N/A

Localizations

  • English
  • Franc
  • Slovak
  • Czech
  • ...

Submitter checklist

  • The Jira / Github issue, if it exists, is well-described.

Maintainer checklist

Before the changes are marked as ready-for-merge:

  • There is at least one (1) approval for the pull request and no outstanding requests for change.
  • Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
  • Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
  • Proper changelog labels are set so that the changelog can be generated automatically.

mPokornyETM and others added 21 commits November 20, 2022 21:21
Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
Co-authored-by: Martin Pokorny <89339813+mPokornyETM@users.noreply.github.com>
@Jagrutiti Jagrutiti requested a review from a team as a code owner November 23, 2022 11:10
@jimklimov
Copy link
Contributor

jimklimov commented Nov 23, 2022

As noted on Gitter, I think it makes better sense to not convert (replace) existing examples, but to really "Add" as in the title, and clearly mark "Example for scripted pipeline" vs. "Example for declarative pipeline". Both dialects are actively used, as different tools for different jobs.

Also, try to constrain the initial PR to focus on its feature changes. Seems it currently includes a version of commits already merged (or proposed elsewhere) to master and they show up in https://github.com/jenkinsci/lockable-resources-plugin/pull/412/files as changes for review: git-rebasing over current upstream/master can help.

@Jagrutiti Jagrutiti changed the base branch from extend-documentation to master November 24, 2022 10:40
@Jagrutiti
Copy link
Contributor Author

Hey @jimklimov

I have added your suggestions and re-based the commit to master.

@jimklimov
Copy link
Contributor

Thanks for the rebase, still the "Changes" look odd. Was [PULL_REQUEST_TEMPLATE.md](https://github.com/jenkinsci/lockable-resources-plugin/pull/412/files#diff-479755504b0aad0fed26047714591511b4d07c76bc8521171b87a5f16100f850) and [org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/index.properties](https://github.com/jenkinsci/lockable-resources-plugin/pull/412/files#diff-812cc60f681b8439c0770757ab5795b69af89070a3c0a5c3e6cd95716821056f) intentional? Or just grabbed by git from that other branch as a difference vs. master?

@mPokornyETM
Copy link
Contributor

When I am correct , this branch was created from extend-documentation
That explain the changes

@mPokornyETM
Copy link
Contributor

I propose to wait for this PR #403 and then re-merge master to this branch. After them you will see real changes.
@Jagrutiti thx for your time

@Jagrutiti
Copy link
Contributor Author

Hey @mPokornyETM , @jimklimov

I had created my branch from extend-documentation. I am using this branch so that we do not face merging issues. I had this discussion with @mPokornyETM.

I wanted you guys to confirm:

  1. Is my script example correct?
  2. Is it alright if I do that same for other examples in the read.me file?
  3. Which branch shall I use as base branch to make my next PR?

@mPokornyETM
Copy link
Contributor

Thx I will wait until #403 is merged. Then it will be easier to review the changes. Pls wait next 1 or 2 days and I give you here a feedback. Thx

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@mPokornyETM mPokornyETM changed the title doc: Added Declarative pipeline syntax (to a few examples) to readme file Added Declarative pipeline example Jan 17, 2023
@mPokornyETM mPokornyETM changed the title Added Declarative pipeline example Added declarative pipeline example Jan 17, 2023
@mPokornyETM mPokornyETM enabled auto-merge (squash) January 17, 2023 16:57
Copy link
Contributor

@mPokornyETM mPokornyETM left a comment

Choose a reason for hiding this comment

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

LGTM

@mPokornyETM mPokornyETM merged commit 283b59e into jenkinsci:master Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[documentation] show example with Declarative syntax
3 participants